New issue
Advanced search Search tips

Issue 873298 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

org.chromium.distiller.DocumentTitleGetterTest.testH1WithLongHTML fails

Project Member Reported by wychen@chromium.org, Aug 10

Issue description

When running DOM distiller tests with https://chromium-review.googlesource.com/c/chromium/src/+/1114673 (with convert_brs_to_newlines set to false), DocumentTitleGetterTest.testH1WithLongHTML fails.

Expected: "long heading with \n5 words"

Actual  : "long heading with 5 words"

Root cause:
In the test, the H1 element is not rendered, but it should in real use case. However, after rendering it, the extracted string becomes "long heading with\n5 words", without the trailing space before the new line. Both behaviors (with or without the trailing space) are acceptable.
 
This fails in latest Firefox as well.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/dom-distiller/+/8825eafdb337c80459bd5048bace9c53e6397b6c

commit 8825eafdb337c80459bd5048bace9c53e6397b6c
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Fri Aug 10 22:52:46 2018

In tests, render elements before getting their innerText

In the tests, make sure an element is rendered before getting its
innerText.

Bug:  651764 ,  873298 
Change-Id: I980a2bdaba85f8226eeba733b47ce53c56bf0dce
Reviewed-on: https://chromium-review.googlesource.com/1171590
Reviewed-by: Matthew Jones <mdjones@chromium.org>

[modify] https://crrev.com/8825eafdb337c80459bd5048bace9c53e6397b6c/javatests/org/chromium/distiller/IEReadingViewParserTest.java
[modify] https://crrev.com/8825eafdb337c80459bd5048bace9c53e6397b6c/javatests/org/chromium/distiller/DocumentTitleGetterTest.java

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 14

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

commit 81db97a9a7f000ed4133a9891781658945853ae3
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Tue Aug 14 00:10:11 2018

Roll DOM Distiller JavaScript distribution package

Diff since last roll:
https://chromium.googlesource.com/chromium/dom-distiller/+/9596033e36..ccfe233400

Picked up changes:
https://chromium.googlesource.com/chromium/dom-distiller/+log/9596033e36..ccfe233400
ccfe233 Fix EmbedExtractorTest.testDivCaption
8825eaf In tests, render elements before getting their innerText
8c9af2c Make generateOutput(textOnly=true) standard compliant

Bug:  651764 , 859410 , 873291 , 873298 
Change-Id: I66bf2c9a433751597fc4c7153ce5955e28dad064
Reviewed-on: https://chromium-review.googlesource.com/1171847
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582763}
[modify] https://crrev.com/81db97a9a7f000ed4133a9891781658945853ae3/DEPS
[modify] https://crrev.com/81db97a9a7f000ed4133a9891781658945853ae3/third_party/dom_distiller_js/README.chromium

Status: Fixed (was: Started)

Sign in to add a comment