New issue
Advanced search Search tips

Issue 697970 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Chrome Dev: For some website (www.cia.gov/index.html), if users download, the offline page misses some contents

Project Member Reported by jiewu@chromium.org, Mar 2 2017

Issue description

Version: 58.0.3026.5
OS: Android 5.0
Device: Samsung Galaxy S5
Model number: SM-G900V
Browser: Chrome Dev

Chrome Dev: For some website (www.cia.gov/index.html), if users download, the offline page misses some contents.

What steps will reproduce the problem?
(1) Internet is on, start Chrome with offline pages enabled
(2) Open a new tab, go to www.cia.gov/index.html
(3) Let the website fully load
(4) Click download button and let the download complete
(5) Open the download, and observe

What is the expected result?
The contents of the offline page should match those of the original webpage. 

What happens instead?
On top of the original webpage, there are “CIA.gov Home” and “Contact us”. These two elements are missing in the offline page. 
Please check out the attached original website and downloaded screenshots to see the difference. 

This bug also exists in the Chrome Dev (58.0.3026.5) and Chrome Beta (57.2.2987.74) in Nexus 7. 

This bug also exists in the Chrome Beta (57.2.2987.74) in Sam Sung Galaxy S5. 
 
offline_Nexus7.JPG
1.6 MB View Download
offline_s5.JPG
1.9 MB View Download
original_Nexus7.JPG
1.6 MB View Download
original_s5.JPG
2.0 MB View Download
Labels: -Restrict-View-Google -Pri-3 Pri-2
Owner: jianli@chromium.org
Status: Assigned (was: Untriaged)
Jian, could you check if this is not us being too aggressive in removing invisible elements during MHTML generation?

Comment 2 by jianli@chromium.org, Apr 14 2017

Labels: M-59
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ddf990431d6b7977a94e5da69f18a1a3d220a52

commit 2ddf990431d6b7977a94e5da69f18a1a3d220a52
Author: jianli <jianli@chromium.org>
Date: Sat Apr 15 03:11:34 2017

Only skip certain hidden elements at MHTML serialization
1) Elements with hidden attributes
2) Hidden form elements

We orginally tried to act more aggresively by skipping all possible
hidden elements. But this does break certain web sites that uses
nth CSS selectors.

BUG= 697970 
TEST=tests updated
TBR=svaldez@chromium.org

Review-Url: https://codereview.chromium.org/2819723003
Cr-Commit-Position: refs/heads/master@{#464856}

[modify] https://crrev.com/2ddf990431d6b7977a94e5da69f18a1a3d220a52/chrome/browser/download/save_page_browsertest.cc
[modify] https://crrev.com/2ddf990431d6b7977a94e5da69f18a1a3d220a52/third_party/WebKit/Source/web/WebFrameSerializer.cpp
[modify] https://crrev.com/2ddf990431d6b7977a94e5da69f18a1a3d220a52/third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp
[modify] https://crrev.com/2ddf990431d6b7977a94e5da69f18a1a3d220a52/third_party/WebKit/Source/web/tests/data/frameserialization/remove_elements.html

Comment 4 by jianli@chromium.org, Apr 17 2017

Labels: Merge-Request-59
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 17 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 17 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/41faf4c8bb4cab7999d14e0a0195cc132beac4e1

commit 41faf4c8bb4cab7999d14e0a0195cc132beac4e1
Author: Jian Li <jianli@chromium.org>
Date: Mon Apr 17 21:19:25 2017

Merge to M59: Only skip certain hidden elements at MHTML serialization
1) Elements with hidden attributes
2) Hidden form elements

We orginally tried to act more aggresively by skipping all possible
hidden elements. But this does break certain web sites that uses
nth CSS selectors.

BUG= 697970 
TEST=tests updated
TBR=svaldez@chromium.org

Review-Url: https://codereview.chromium.org/2819723003
Cr-Commit-Position: refs/heads/master@{#464856}
(cherry picked from commit 2ddf990431d6b7977a94e5da69f18a1a3d220a52)

Review-Url: https://codereview.chromium.org/2826533002 .
Cr-Commit-Position: refs/branch-heads/3071@{#25}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/41faf4c8bb4cab7999d14e0a0195cc132beac4e1/chrome/browser/download/save_page_browsertest.cc
[modify] https://crrev.com/41faf4c8bb4cab7999d14e0a0195cc132beac4e1/third_party/WebKit/Source/web/WebFrameSerializer.cpp
[modify] https://crrev.com/41faf4c8bb4cab7999d14e0a0195cc132beac4e1/third_party/WebKit/Source/web/tests/WebFrameSerializerTest.cpp
[modify] https://crrev.com/41faf4c8bb4cab7999d14e0a0195cc132beac4e1/third_party/WebKit/Source/web/tests/data/frameserialization/remove_elements.html

Comment 7 by jianli@chromium.org, Apr 17 2017

Status: Fixed (was: Assigned)
Verified on M59 -59.0.3071.15 build

Sign in to add a comment