1248381 - Inspector panel should display properly capitalized node name

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[Nicolas Chevobbe [:nchevobbe]]

Attachment: uncapitalizedMarkup.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160209234513

Steps to reproduce:

1. Open the attached file
2. Open the devtools
3. Go to inspector tab


Actual results:

Markup is displayed, with a <clippath> element


Expected results:

The element should be properly capitalized : <clipPath>

It may be confusing to see <clippath>, because you need to write document.querySelector('clipPath') to get the element. document.querySelector('clippath') will return "null"

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Could you assign it to me if the bug is valid ? Thank you

Comment 2

[Julian Descottes [:jdescottes]]

Thanks for opening the bug! Assigning to you.

In case you need a pointer to start, tag names are lower-cased in the constructor of ElementEditor, in markup.js.

Comment 3

[Julian Descottes [:jdescottes]]

Nicolas: As discussed on IRC, we have a similar issue for XHTML documents. The markup view should not lowercase tag names and attribute names when rendering the markup of an XHTML doc. 

Keep it in mind when fixing this issue, maybe both issues can be addressed here. If not we will deal with this in a separate bug.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

> Nicolas: As discussed on IRC, we have a similar issue for XHTML documents. The markup view should not lowercase tag names and attribute names when rendering the markup of an XHTML doc. 

Should it be the case for html elements too ? 

I dug a little and it turns out that displaying the element in the console preserve the case, at least for svg elements, thanks to https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/object.js#1490

So I was wondering if we could do something similar to : 

let nodeName = node.nodeName;
if( nodeName instanceof HTMLElement ) {
  nodeName = nodeName.toLowerCase();
}

But I don't know if it would handle the XHTML issue you're talking here.
Moreover, in the snippet above, node is not a Node element, but a NodeActor, and the rawNode property doesn't seems accessible, nor recommended ( https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/inspector.js#1097 ) so it won't be possible to test on the client side if it is actually an HTMLElement.
Maybe we should do it like in the object actor, and have NodeActor.nodeName properly capitalized ? It may be possible that some functions rely on the fact that NodeActor.nodeName is equal to NodeActor.rawNode.nodeName though.

Comment 5

[Julian Descottes [:jdescottes]]

As discussed on IRC, I think the lowercase() should be done on the client side. 

We can check the namespace of the node to know if it's SVG or not. Won't work for XHTML documents, but let's leave this for later.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

nodeName will always be the correct case... you don't need to use toLowerCase() to display it.

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #6)
> nodeName will always be the correct case... you don't need to use
> toLowerCase() to display it.

For svg elements, yes it will be in the correct case ( <cliPath>.nodeName => 'clipPath').
But for html ones, on an HTML document, we have to call toLowercase ( <div>.nodeName => 'DIV' )

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Attachment: MozReview Request: Bug 1248381 - Inspector panel should display properly capitalized node name; r?miker

Review commit: https://reviewboard.mozilla.org/r/35281/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35281/

Comment 9

[Julian Descottes [:jdescottes]]

Nicolas: Thanks for the review request! However, I am not a peer for this module and can't officially review your patch.
Can you switch the review request to Mike?

(I will still test the patch and give informal feedback)

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8720363 [details]
MozReview Request: Bug 1248381 - Inspector panel should display properly capitalized node name; r?miker

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35281/diff/1-2/

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Julian Descottes [:jdescottes] from comment #9)
> Nicolas: Thanks for the review request! However, I am not a peer for this
> module and can't officially review your patch.
> Can you switch the review request to Mike?
> 
> (I will still test the patch and give informal feedback)

Oops, sorry about that.

Comment 12

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8720363 [details]
MozReview Request: Bug 1248381 - Inspector panel should display properly capitalized node name; r?miker

https://reviewboard.mozilla.org/r/35281/#review32067

Perfect... thanks for working on this.

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Do you want me to push to try ?

Comment 14

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Sure, you can do that from Review Board now. Just go to https://reviewboard.mozilla.org/r/35281/#review32067, click automation and then trigger a try build.

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

Here it is : 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f904292f5a4a

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

pulse went down during the test ( see https://bugzilla.mozilla.org/show_bug.cgi?id=1249272 )
New push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=29d8dc48e349

Comment 17

[Nicolas Chevobbe [:nchevobbe]]

The job is over, there are some errors but none seems related to this patch.

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/1a1ec353998f

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1a1ec353998f

Comment 21

[Kazi Nuzhat Tasnem]

I have reproduced this bug with Firefox nightly 47.0a1(build id-20160209234513)on
windows 7(64 bit)

I have verified this bug as fixed with Firefox aurora 48.0a2(build id-20160606004039)
User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

[bugday-20160617]

Comment 22

[Khalid Syfullah Zaman [:khalid32]]

I have reproduced this bug on Nightly 47.0a1 (2016-02-15) (Build ID: 20160215030213) on Linux, 64 bit!

The bug's fix is now verified on Latest Beta 48.0b4!

Build ID: 20160627144420
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0
OS: Ubuntu 16.04, Linux 4.4.0-28-generic

As this bug' fix is also verified on Windows 7, 64 bit(comment 21), I am marking this as verified!