MHTML does not preserve the @font-face specified as data: url |
||||||||||||
Issue descriptionin 54.0.2840.25 Android Chrome, or in 54.0.2840.27 on Windows, or similar on Linux: 1. Open attached file test.html. Observe twitter icon rendered from embedded font. 2. Save the page as MHTML (on desktop, enable MHTML save in chrome://flags), on Android, click Download button 3. Open the saved page, observe the computer icon (on Linux) or box (on Android) instead of a twitter icon. See expected.png, observed.png, test.html and test.mhtml attached.
,
Sep 15 2016
,
Sep 15 2016
I am guessing that sandboxing JS also disallows inline style tags.
,
Sep 16 2016
,
Sep 16 2016
I verified that this is unrelated to the sandbox.
,
Sep 16 2016
This appears related to the security fix added by jianli@ not too long ago in crrev.com/7f5d61af3ec5b0ec55f708111e5845aee0886cba For MTHML archives, we force all resource requests through ResourceFetcher::resourceForStaticData which calls Platform::parseDataURL when it encounters a data: URL. parseDataURL only returns data for valid URLs which are "supported". I am not quite sure about this but I think "supported" means that Blink knows how to render this in a frame, something like HTML or an image. Fonts are not "supported". So, parseDataURL returns no data for any fonts in the data: scheme. resourceForStaticData is the only caller of parseDataURL, so we could modify this (with the exception that it is a part of the Platform.h api... is that important?). japhet@ - do you know if the "supported"-ness of data URLs is relevant to this code path? You are in the blamelist for initially adding parseDataURL to this section (https://codereview.chromium.org/1291883002).
,
Sep 16 2016
It doen't seem rigth that fonts encoded as data: urls can't be displayed in an archive. I don't think there's anything magical about parseDataURL() being part of the Platform.h api. parseDataURL() should probably be permitted to handle data: urls, though I don't know enough about what's going on outside of blink to know whether this should be fixed, e.g., in mime_type.cc
,
Sep 16 2016
I wonder why have a check for mime types in parseDataURL at all - if it fails, the request just progresses through regular loading and will be converted to the resource anyways. It appears that removal of this check is a functional noop for anything except MHTML archive - and only fixes the archive case.
,
Sep 16 2016
Removing the mime-type check appears to cause inline fonts with data: urls to be decoded even if they are never used (this makes some sense). There are layout tests that explicitly check for this.
,
Sep 16 2016
Because of the issue in #10, I believe the correct strategy is to allow data URLs to be processed using the normal request machinery even in an archive.
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4204367ed53fb3ba794678d78e7d4fc149f17787 commit 4204367ed53fb3ba794678d78e7d4fc149f17787 Author: dewittj <dewittj@chromium.org> Date: Thu Sep 22 16:15:54 2016 MHTML: Allows 'data:' URLs to be processed using normal request processing. An alternative of allowing Blink to parse data URLs for all content-types was considered, but this breaks behavior elsewhere in blink that requires fonts defined in 'data:' URLs but not referenced never be decoded. Instead, we stop sandboxing requests for 'data:' URLs and allow them to use the normal request machinery. BUG= 647122 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:win7_blink_dbg,linux_precise_blink_rel,linux_precise_blink_dbg,mac10.9_blink_dbg,mac10.9_blink_rel,win7_blink_rel,mac10.10_blink_rel,mac10.11_blink_rel,mac10.11_retina_blink_rel,win10_blink_rel,linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2346293002 Cr-Commit-Position: refs/heads/master@{#420365} [modify] https://crrev.com/4204367ed53fb3ba794678d78e7d4fc149f17787/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/4204367ed53fb3ba794678d78e7d4fc149f17787/third_party/WebKit/LayoutTests/mhtml/data-uri-font.mht [add] https://crrev.com/4204367ed53fb3ba794678d78e7d4fc149f17787/third_party/WebKit/LayoutTests/platform/linux/mhtml/data-uri-font-expected.png [add] https://crrev.com/4204367ed53fb3ba794678d78e7d4fc149f17787/third_party/WebKit/LayoutTests/platform/linux/mhtml/data-uri-font-expected.txt [modify] https://crrev.com/4204367ed53fb3ba794678d78e7d4fc149f17787/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
,
Sep 22 2016
,
Sep 22 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e3673322e139ac2d5504474e6a62f5404d78dca5 commit e3673322e139ac2d5504474e6a62f5404d78dca5 Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org> Date: Thu Sep 22 17:28:30 2016 Auto-rebaseline for r420365 https://chromium.googlesource.com/chromium/src/+/4204367ed BUG= 647122 TBR=dewittj@chromium.org Review URL: https://codereview.chromium.org/2364663002 . Cr-Commit-Position: refs/heads/master@{#420384} [modify] https://crrev.com/e3673322e139ac2d5504474e6a62f5404d78dca5/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/e3673322e139ac2d5504474e6a62f5404d78dca5/third_party/WebKit/LayoutTests/mhtml/data-uri-font-expected.png [add] https://crrev.com/e3673322e139ac2d5504474e6a62f5404d78dca5/third_party/WebKit/LayoutTests/platform/mac/mhtml/data-uri-font-expected.txt [add] https://crrev.com/e3673322e139ac2d5504474e6a62f5404d78dca5/third_party/WebKit/LayoutTests/platform/win/mhtml/data-uri-font-expected.txt
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/329811b9f577cc8f1484ba82a46dfdb91d95fdcd commit 329811b9f577cc8f1484ba82a46dfdb91d95fdcd Author: Justin DeWitt <dewittj@chromium.org> Date: Thu Sep 22 17:50:01 2016 MHTML: Allows 'data:' URLs to be processed using normal request processing. An alternative of allowing Blink to parse data URLs for all content-types was considered, but this breaks behavior elsewhere in blink that requires fonts defined in 'data:' URLs but not referenced never be decoded. Instead, we stop sandboxing requests for 'data:' URLs and allow them to use the normal request machinery. BUG= 647122 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:win7_blink_dbg,linux_precise_blink_rel,linux_precise_blink_dbg,mac10.9_blink_dbg,mac10.9_blink_rel,win7_blink_rel,mac10.10_blink_rel,mac10.11_blink_rel,mac10.11_retina_blink_rel,win10_blink_rel,linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2346293002 Cr-Commit-Position: refs/heads/master@{#420365} (cherry picked from commit 4204367ed53fb3ba794678d78e7d4fc149f17787) Review URL: https://codereview.chromium.org/2359963003 . Cr-Commit-Position: refs/branch-heads/2840@{#493} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/mhtml/data-uri-font.mht [add] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/platform/linux/mhtml/data-uri-font-expected.png [add] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/platform/linux/mhtml/data-uri-font-expected.txt [modify] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
,
Sep 23 2016
,
Sep 28 2016
This issue is now not reproducible on latest M54-54.0.2840.42 on android devices, verified on Samsung Galaxy S6(SM-G920I)/MMB29K and Moto G4/6.0.1
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/329811b9f577cc8f1484ba82a46dfdb91d95fdcd commit 329811b9f577cc8f1484ba82a46dfdb91d95fdcd Author: Justin DeWitt <dewittj@chromium.org> Date: Thu Sep 22 17:50:01 2016 MHTML: Allows 'data:' URLs to be processed using normal request processing. An alternative of allowing Blink to parse data URLs for all content-types was considered, but this breaks behavior elsewhere in blink that requires fonts defined in 'data:' URLs but not referenced never be decoded. Instead, we stop sandboxing requests for 'data:' URLs and allow them to use the normal request machinery. BUG= 647122 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:win7_blink_dbg,linux_precise_blink_rel,linux_precise_blink_dbg,mac10.9_blink_dbg,mac10.9_blink_rel,win7_blink_rel,mac10.10_blink_rel,mac10.11_blink_rel,mac10.11_retina_blink_rel,win10_blink_rel,linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2346293002 Cr-Commit-Position: refs/heads/master@{#420365} (cherry picked from commit 4204367ed53fb3ba794678d78e7d4fc149f17787) Review URL: https://codereview.chromium.org/2359963003 . Cr-Commit-Position: refs/branch-heads/2840@{#493} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/mhtml/data-uri-font.mht [add] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/platform/linux/mhtml/data-uri-font-expected.png [add] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/LayoutTests/platform/linux/mhtml/data-uri-font-expected.txt [modify] https://crrev.com/329811b9f577cc8f1484ba82a46dfdb91d95fdcd/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by dim...@chromium.org
, Sep 15 201662.1 KB
62.1 KB View Download