983614 - Output of newly styled DOM nodes in console buggy.

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Dane MacMillan [:danemacmillan]]

This bug is related to bug 682033. That bug enhanced the output of nodes in the console. It works great, with the exception of the output you see in the image:

http://i.imgur.com/eHYZMH1.png

There are an excess of periods following one of the class names on the body. For the meantime I seem only to reproduce it on the bugzilla pages. I'm sure it can be reproduced elsewhere. Maybe it's the colon in the class name that is affecting the output.

Comment 1

[Mihai Sucan [:msucan]]

Patrick: it seems we dont cleanup className before split(' ').

Comment 2

[Patrick Brosset <:pbro>]

Attachment: bug983614-cleanup-classnames-in-domnode-console-output v1.patch

Instead of splitting on " ", now splitting on regexp \s+
Added a new DOM element in one of the test files to ensure this doesn't happen again.

Comment 3

[Patrick Brosset <:pbro>]

ongoing try build: https://tbpl.mozilla.org/?tree=Try&rev=6a3451414b21

Comment 4

[Mihai Sucan [:msucan]]

Comment on attachment 8393405 [details] [diff] [review]
bug983614-cleanup-classnames-in-domnode-console-output v1.patch

Review of attachment 8393405 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Do we want this fix in aurora? If you think we should get this in aurora, please also ask for approval.

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8393405 [details] [diff] [review]
bug983614-cleanup-classnames-in-domnode-console-output v1.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug is a follow-up of bug 757866 which is a new feature that made it to Aurora 30 last week.
This new feature enriches the web console output for DOM Nodes.

User impact if declined: If declined, DOM nodes returned by querySelectorAll displayed in the web console output may not be very pretty in some cases: there may be too many dots between classnames as shown in the screenshot attached.

Testing completed (on m-c, etc.): new bc mochitest added, ongoing try build: https://tbpl.mozilla.org/?tree=Try&rev=6a3451414b21
But not yet landed in m-c. 

Risk to taking this patch (and alternatives if risky): this patch is only changes a `className.split(" ")` into `className.split(/\s+/g)` and only concern DOMNodes being output into the webconsole (when part of NodeList only), so the potential impacts of this patch are very very limited.

String or IDL/UUID changes made by this patch: None

Comment 6

[Patrick Brosset <:pbro>]

I was a bit too quick in asking for check-in, the try build isn't even finished ... https://tbpl.mozilla.org/?tree=Try&rev=6a3451414b21

Comment 7

[Patrick Brosset <:pbro>]

Green try build and R+
Fixed in fx-team: https://hg.mozilla.org/integration/fx-team/rev/7cfac9ff3b02

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7cfac9ff3b02

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b4d43595fd58

Comment 10

[u279076]

Looks like this landed with tests. Please correct if I am mistaken.