1768161 - Browser Toolbox Rules tab incorrectly shows height and width as invalid for img elements

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Henry Wilkes (they/them) [:henry-x]]

Steps to reproduce.

1. Open the Browser Toolbox.
2. Find an <img> element whose computed display has not been changed from the default inline.
3. Inspect its CSS "Rules" and set its width or height.

Result

The width and height properties will be greyed out with an "info" icon that says "width has no effect on this element since it has a display of inline", or the equivalent for "height".

Expect

The width and height properties are valid for replaced elements like <img>, so shouldn't be shown as invalid. Note, this works fine when using the "Web Developer Tools" inspector for <img> elements on web pages.

Comment 1

[Julian Descottes [:jdescottes]]

Thanks for filing, can you share more details? I tried inspecting and modifying img tags in the Browser Chrome and I can't reproduce the issue.
Is this something you see when debugging thunderbird?

Comment 2

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Julian Descottes [:jdescottes] from comment #1)

Thanks for filing, can you share more details? I tried inspecting and modifying img tags in the Browser Chrome and I can't reproduce the issue.
Is this something you see when debugging thunderbird?

This does come up more in Thunderbird because we have a lot of img elements. But I can reproduce using firefox tip as well:

1. Open firefox.
2. Open the Browser Toolbar.
3. Find and select a .tab-icon-stack in the Inspector.
4. In the Rules tab, add the rule display: block;. This is needed because the original grid display treats the child img as a grid item, causing it to loose its computed inline display.
5. Select the child .tab-icon-image.
6. Scroll to the CSS rule from "tabs.css" that sets the height and width to 16px. These two rules will have invalid icons next to them.

Comment 3

[Julian Descottes [:jdescottes]]

Perfect thanks! Probably an issue around https://searchfox.org/mozilla-central/rev/9f95c41a962c9228f569f8a6b2c30edbb50b65ae/devtools/server/actors/utils/inactive-property-helper.js#839

Comment 4

[Julian Descottes [:jdescottes]]

Our logic checks for nodeName, which in this case is html:img, whereas we assume it would be img

Comment 5

[Julian Descottes [:jdescottes]]

Let's use localName instead.

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1768161 - [devtools] Only export isPropertyUsed from inactive-property-helper.js

It was not clear which methods/getters were internal or not.
Exposing only the helper method helps with refactoring

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1768161 - [devtools] Use localName instead of nodeName in inactive-property-helper.js

Depends on D146898
nodeName can include the namespace prefix eg html:img, failing some of our string comparisons

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c638dd0fa366
[devtools] Only export isPropertyUsed from inactive-property-helper.js r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/51a3fc9e4d16
[devtools] Use localName instead of nodeName in inactive-property-helper.js r=nchevobbe

Comment 9

[Cristian Tuns]

Sorry about ni, not indented

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c638dd0fa366
https://hg.mozilla.org/mozilla-central/rev/51a3fc9e4d16