Closed Bug 1436185 Opened 2 years ago Closed 2 years ago

CSS Rules Inspector has inconsistently (/ increasingly) indented declarations

Categories

(DevTools :: Inspector: Rules, defect, P3)

All
Linux
defect

Tracking

(firefox-esr52 unaffected, firefox58 unaffected, firefox59 unaffected, firefox60+ fixed)

RESOLVED FIXED
Firefox 60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 + fixed

People

(Reporter: dholbert, Assigned: julienw)

References

Details

(Keywords: regression)

Attachments

(4 files)

Attached image screenshot
STR:
 1. Visit https://bug1436180.bmoattachments.org/attachment.cgi?id=8948795
 2. Right click one of the gray placeholder images and choose "Inspect Element"
 3. Look at the rules view.

ACTUAL RESULTS: It looks like this:
> #slides > div, #slides > div > img {
>     height: 100%;
>       vertical-align: top;
>         width: auto;
>           }

EXPECTED RESULTS:
Declarations shouldn't be progressively indented like that.

I'm using Nightly on Ubuntu 16.10.
Nightly version: 60.0a1 (2018-02-05) (64-bit)

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1142fe8c0a2e61a2ee73e70bb805bfeaac44d595&tochange=1afdba933156fc334921e7924e5edca02050e295

--> regression from bug 1403334


This might be linux-specific -- jdescottes says he's unable to reproduce on Mac, at least.
(Requesting tracking for version 60, as this is a pretty visible UI regression which we should avoid shipping if at all possible.)
Gabe or Liam, could you take a look at this?
Flags: needinfo?(gl)
I was able to reproduce this on linux as well, the rule view indent is broken for any document, not just mozilla.org or comment 0 website.

It seems to be related to the "float: left;" on .ruleview-enableproperty, introduced by bug 1403334.
I also tried on windows and the indent is correct.
I have no idea why it is linux specific nor what is wrong with this float.
(In reply to Alexandre Poirot [:ochameau] from comment #4)
> I was able to reproduce this on linux as well

Great! Glad it's not just me. :)

> the rule view indent is
> broken for any document, not just mozilla.org or comment 0 website.

(Right. Sorry for not making that clearer. This affects the rules inspector in general - not just for specific sites.)

> I also tried on windows and the indent is correct.
> I have no idea why it is linux specific nor what is wrong with this float.

I also tried Windows & can't reproduce.  And I'm also unable to reproduce on a different Linux device (my ThinkPad), running the same version of Ubuntu as the desktop device where I can reproduce.  So there might be some subtle system-configuration dependency here (maybe specific font, display / resolution, etc).
(I'm guessing there might be a fragile dependency on some element(s) being exactly as wide as its container -- and in certain circumstances there's a rounding difference and it's too wide, and something gets wrapped to the next line.)
I can also reproduce on my Ubuntu 16.04 machine.  I am happy to help if needed, but I don't entirely follow what the regressing bug was trying to accomplish, since both the original bug and commit are quite vague. :X Let me know if there's something I can do.
Attached image After vs Before Indent
So, this is probably caused by Bug 1403334. You can see the original intent of the bug in the attached screenshot. The top is after versus the bottom before of how it looks for the property value text overflow are indent. 

This is probably linux specific and I don't currently have a linux bootcamp to examine this. Would anyone else be available to check what is happening in the CSS here?
Flags: needinfo?(gl)
Adding a "clear: left" to that rule for ".ruleview-enableproperty" fixes the issue for me on Linux. I don't know if this is the right fix though.
Note: my colleague Jérémie got the issue on Mac so it's not Linux specific.
Forcing a line-height of 14px to ruleview-property fixes the issue for me too. Otherwise it's computed to 13.5px which is smaller to the size of the floated element. I'd suggest to do this so that we don't have similar bugs in the future.

As a side note, maybe using "display: flex" would have been more robust :-)
`clear: left` seems like a reasonable short term fix we can land quickly.  Julien, do you want to prepare the patch?
Flags: needinfo?(felash)
OK, taking !
Assignee: nobody → felash
Flags: needinfo?(felash)
Priority: -- → P3
Comment on attachment 8949016 [details]
Bug 1436185 - Fix indentation in the rules subpanel in the inspector

https://reviewboard.mozilla.org/r/218414/#review224308
Attachment #8949016 - Flags: review?(gl) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e49f9b8dad9f
Fix indentation in the rules subpanel in the inspector r=gl
Blocks: 1433716
No longer blocks: 1433716
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/e49f9b8dad9f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.