Inspector panel should display properly capitalized node name

VERIFIED FIXED in Firefox 47

Status

VERIFIED FIXED
3 years ago
9 months ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

({testcase})

44 Branch
Firefox 47
testcase

Firefox Tracking Flags

(firefox47 fixed, firefox48 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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"
(Assignee)

Comment 1

3 years ago
Could you assign it to me if the bug is valid ? Thank you
Component: Untriaged → Developer Tools: Inspector
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.
Assignee: nobody → chevobbe.nicolas

Updated

3 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: testcase
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.
(Assignee)

Comment 4

3 years ago
> 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.
Flags: needinfo?(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.
Flags: needinfo?(jdescottes)
nodeName will always be the correct case... you don't need to use toLowerCase() to display it.
(Assignee)

Comment 7

3 years ago
(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' )
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)
Flags: needinfo?(chevobbe.nicolas)
(Assignee)

Comment 10

3 years ago
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/
Attachment #8720363 - Attachment description: MozReview Request: Bug 1248381 - Inspector panel should display properly capitalized node name; r?jdescottes → MozReview Request: Bug 1248381 - Inspector panel should display properly capitalized node name; r?miker
Attachment #8720363 - Flags: review?(jdescottes) → review?(mratcliffe)
(Assignee)

Comment 11

3 years ago
(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.
Flags: needinfo?(chevobbe.nicolas)
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.
Attachment #8720363 - Flags: review?(mratcliffe) → review+
(Assignee)

Comment 13

3 years ago
Do you want me to push to try ?
Flags: needinfo?(mratcliffe)
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.
Flags: needinfo?(mratcliffe)
(Assignee)

Comment 17

3 years ago
The job is over, there are some errors but none seems related to this patch.
Keywords: checkin-needed

Comment 19

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1a1ec353998f
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
(Assignee)

Updated

3 years ago
Attachment #8720363 - Attachment is obsolete: true

Updated

3 years ago
Duplicate of this bug: 831395
(Assignee)

Updated

3 years ago
See Also: → bug 1270205
(Assignee)

Updated

3 years ago
See Also: → bug 1270215
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]
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!
Status: RESOLVED → VERIFIED
QA Whiteboard: [bugday-20160629]
status-firefox48: --- → verified

Updated

9 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.