Fix more partially hidden articles
When traversing the DOM tree, invisible elements are ignored. In order
to correctly handle partially hidden articles on sites having the
"continue reading" button, if the class name contains "continue", it
would still be processed.
BUG=687071
R=mdjones@chromium.org
Review-Url: https://codereview.chromium.org/2669723002 .
diff --git a/java/org/chromium/distiller/webdocument/DomConverter.java b/java/org/chromium/distiller/webdocument/DomConverter.java
index 0157fbc..56fee56 100644
--- a/java/org/chromium/distiller/webdocument/DomConverter.java
+++ b/java/org/chromium/distiller/webdocument/DomConverter.java
@@ -96,17 +96,23 @@
boolean keepAnyway = false;
boolean hasHiddenClassName = false;
if (!visible) {
+ // Process more hidden elements in a marked article in mobile-friendly pages
+ // because some sites hide the lower part of the article.
if (isMobileFriendly && hasArticleElement) {
if (!isHiddenClass) {
hasHiddenClassName = DomUtil.hasClassName(e, "hidden");
}
if (isHiddenClass || hasHiddenClassName) {
- // Process more hidden elements in a marked article in mobile-friendly pages
- // because some sites hide the lower part of the article.
// See crbug.com/599121
keepAnyway = true;
}
}
+ if (isMobileFriendly) {
+ if (e.getAttribute("class").contains("continue")) {
+ // See crbug.com/687071
+ keepAnyway = true;
+ }
+ }
}
logVisibilityInfo(e, visible || keepAnyway);
if (!visible && !keepAnyway) {
diff --git a/javatests/org/chromium/distiller/webdocument/DomConverterTest.java b/javatests/org/chromium/distiller/webdocument/DomConverterTest.java
index 0fa94bb..a4c6f64 100644
--- a/javatests/org/chromium/distiller/webdocument/DomConverterTest.java
+++ b/javatests/org/chromium/distiller/webdocument/DomConverterTest.java
@@ -14,11 +14,18 @@
public class DomConverterTest extends DomDistillerJsTestCase {
private void runTest(String innerHtml, String expectedHtml) throws Throwable {
+ runTest(innerHtml, expectedHtml, false);
+ }
+
+ private void runTest(String innerHtml, String expectedHtml, boolean mobileArticle)
+ throws Throwable {
Element container = Document.get().createDivElement();
mBody.appendChild(container);
container.setInnerHTML(innerHtml);
FakeWebDocumentBuilder builder = new FakeWebDocumentBuilder();
DomConverter filteringDomVisitor = new DomConverter(builder);
+ filteringDomVisitor.setHasArticleElement(mobileArticle);
+ filteringDomVisitor.setIsMobileFriendly(mobileArticle);
new DomWalker(filteringDomVisitor).walk(container);
String expectedDocument = "<div>" + expectedHtml + "</div>";
assertEquals(expectedDocument, builder.getDocumentString().toLowerCase());
@@ -51,8 +58,15 @@
public void testVisibleInInvisible() throws Throwable {
String html = "<div style=\"visibility:hidden\">invisible parent" +
- "<div>visible child</div>" +
- "</div>";
+ "<div>visible child</div>" +
+ "</div>";
+ runTest(html, "");
+ }
+
+ public void testVisibleInInvisible2() throws Throwable {
+ String html = "<div style=\"display:none\">invisible parent" +
+ "<div>visible child</div>" +
+ "</div>";
runTest(html, "");
}
@@ -92,6 +106,30 @@
runTest(html, "");
}
+ public void testKeepHidden() throws Throwable {
+ String html = "<div class=\"hidden\" style=\"display: none\">visible element</div>";
+ runTest(html, html, true);
+ }
+
+ public void testKeepHiddenNested() throws Throwable {
+ // "visibility: hidden" is used. See crbug.com/599121
+ String html = "<div class=\"hidden\" style=\"visibility: hidden\">" +
+ "<div>visible element</div></div>";
+ runTest(html, html, true);
+ }
+
+ public void testKeepContinue() throws Throwable {
+ String html = "<div class=\"continue\" style=\"display: none\">visible element</div>";
+ runTest(html, html, true);
+ }
+
+ public void testKeepContinueNested() throws Throwable {
+ // "display: none" is used. See crbug.com/687071
+ String html = "<div class=\"continue\" style=\"display: none\">" +
+ "<div>visible element</div></div>";
+ runTest(html, html, true);
+ }
+
public void testDataTable() throws Throwable {
String html = "<table align=\"left\" role=\"grid\">" + // role=grid make this a data table.
"<tbody align=\"left\">" +
