Strip unwanted classNames from all nodes

Strip unwanted classNames from all nodes

Class name "caption" is still essential for styling, but all the other
classNames are removed. Ref: crbug.com/654109

BUG=599121
R=mdjones@chromium.org

Committed: 3fd2017fe4684a67114daf93bc652166f50f865b

[wychen, 2016-10-07 21:43:44]

wychen@chromium.org changed reviewers:
+ mdjones@chromium.org

[wychen, 2016-10-07 21:43:44]

PTAL

[mdjones, 2016-10-07 23:03:37]

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
File java/org/chromium/distiller/DomUtil.java (right):

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
java/org/chromium/distiller/DomUtil.java:477: * Strips unwanted classNames from
all nodes in the tree rooted at |root|.
Can you mention here and in the CL description what you mean by "unwanted class
names"? This function does something very specific.

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
java/org/chromium/distiller/DomUtil.java:486: NodeList<Element> elems =
DomUtil.querySelectorAll(root, "[class]");
Why not select all elements and iterate over the resulting list?

[wychen, 2016-10-07 23:11:53]

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
File java/org/chromium/distiller/DomUtil.java (right):

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
java/org/chromium/distiller/DomUtil.java:477: * Strips unwanted classNames from
all nodes in the tree rooted at |root|.
On 2016/10/07 23:03:37, mdjones wrote:
> Can you mention here and in the CL description what you mean by "unwanted
class
> names"? This function does something very specific.

Will do.

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
java/org/chromium/distiller/DomUtil.java:486: NodeList<Element> elems =
DomUtil.querySelectorAll(root, "[class]");
On 2016/10/07 23:03:37, mdjones wrote:
> Why not select all elements and iterate over the resulting list?

Not all elements have class attribute, so skipping them using querySelectorAll
should be much faster.

[wychen, 2016-10-07 23:30:26]

Description was changed from

==========
Strip unwanted classNames from all nodes

BUG=599121
==========

to

==========
Strip unwanted classNames from all nodes

Class name "caption" is still essential for styling, but all the other
classNames are
removed. Ref: crbug.com/654109

BUG=599121
==========

[wychen, 2016-10-07 23:31:21]

Description was changed from

==========
Strip unwanted classNames from all nodes

Class name "caption" is still essential for styling, but all the other
classNames are
removed. Ref: crbug.com/654109

BUG=599121
==========

to

==========
Strip unwanted classNames from all nodes

Class name "caption" is still essential for styling, but all the other
classNames are removed. Ref: crbug.com/654109

BUG=599121
==========

[wychen, 2016-10-07 23:32:20]

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
File java/org/chromium/distiller/DomUtil.java (right):

https://codereview.chromium.org/2401853004/diff/1/java/org/chromium/distiller...
java/org/chromium/distiller/DomUtil.java:477: * Strips unwanted classNames from
all nodes in the tree rooted at |root|.
On 2016/10/07 23:11:52, wychen wrote:
> On 2016/10/07 23:03:37, mdjones wrote:
> > Can you mention here and in the CL description what you mean by "unwanted
> class
> > names"? This function does something very specific.
> 
> Will do.

Done.

[mdjones, 2016-10-07 23:37:00]

lgtm

[wychen, 2016-10-11 02:00:18]

Description was changed from

==========
Strip unwanted classNames from all nodes

Class name "caption" is still essential for styling, but all the other
classNames are removed. Ref: crbug.com/654109

BUG=599121
==========

to

==========
Strip unwanted classNames from all nodes

Class name "caption" is still essential for styling, but all the other
classNames are removed. Ref: crbug.com/654109

BUG=599121
R=mdjones@chromium.org

Committed: 3fd2017fe4684a67114daf93bc652166f50f865b
==========

[wychen, 2016-10-11 02:00:20]

Committed patchset #2 (id:20001) manually as
3fd2017fe4684a67114daf93bc652166f50f865b (presubmit successful).