New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 676638 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

False positive of comment-detecting heuristics in Dom distiller

Project Member Reported by olivierrobin@chromium.org, Dec 22 2016

Issue description

AMP pages are currently not distilled by DOM distiller on iOS.
AMP pages are growing and we need to integrate them.

 

Comment 2 by wychen@chromium.org, Dec 23 2016

Summary: False positive of comment-detecting heuristics in Dom distiller (was: Dom distiller and AMP pages)
It's not actually about AMP.

The root cause is that the <div> containing the main content has class="content__main tonal__main tonal__main--tone-comment", which triggers a heuristic saying it looks like the comment section.

Comment 3 by wychen@chromium.org, Dec 23 2016

Cc: mdjones@chromium.org k...@chromium.org

Comment 4 by wychen@chromium.org, Dec 23 2016

Status: Started (was: Assigned)
The fix is here:
https://codereview.chromium.org/2596283004/
Great. Thank you, Wei-Yin!
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 9 2017

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

commit b6bda67c0ddcc984c0f27da39b95fa018fcde652
Author: wychen <wychen@chromium.org>
Date: Mon Jan 09 18:11:00 2017

Roll DOM Distiller JavaScript distribution package

Diff since last roll:
https://github.com/chromium/dom-distiller/compare/072fe57b48...aab1a1b038

Picked up changes:
aab1a1b Revert "Replace create_standalone_js.py with a custom GWT linker"
ac91168 Use stricter comment-detecting heuristics
936bafa Skip <svg> elements when scanning the DOM
315e460 Skip lead image finder when no text content

BUG= 617360 , 658113 , 676638 

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

[modify] https://crrev.com/b6bda67c0ddcc984c0f27da39b95fa018fcde652/DEPS
[modify] https://crrev.com/b6bda67c0ddcc984c0f27da39b95fa018fcde652/third_party/dom_distiller_js/README.chromium

Comment 7 by wychen@chromium.org, Jan 10 2017

Status: Fixed (was: Started)
Testing in 58.0.2991.0 dev:
Example URL from comment #1: 
https://www.google.fr/amp/s/amp.theguardian.com/commentisfree/2016/nov/15/rust-belt-middle-class-wiped-out

this url is not getting distilled is this WAI?
I think Olivier just submitted a fix here:
https://bugs.chromium.org/p/chromium/issues/detail?id=684554#c3
Yes, I just submitted a "fix". We will test in tomorrow canary.
As the problem is a timing issue, behavior on release may be different from my dev build.
Tested on today canary. Page was distilled correctly.
Status: Verified (was: Fixed)
Verified in 58.0.2998.0 canary, iPhone 6+ iOS 10.2, iPad mini 10.1

Tested the URL in comment #8 and other search pages, searches are getting distilled. Looks good.

Sign in to add a comment