Closed Bug 1583034 Opened 2 years ago Closed 2 years ago

The horizontal scroll bar is always displayed in the Markup View

Categories

(DevTools :: Inspector, defect, P2)

71 Branch
defect

Tracking

(firefox-esr60 unaffected, firefox-esr68 unaffected, firefox67 unaffected, firefox68 unaffected, firefox69 unaffected, firefox70 unaffected, firefox71 fixed)

RESOLVED FIXED
Firefox 71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- fixed

People

(Reporter: nayinain, Assigned: fvsch)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

  1. Inspect Element on the about:blank.

Actual results:

The horizontal scroll bar appear in the markup view.

Expected results:

The horizontal scroll bar should not appear in markup view.

Attached video Capture.webm

###regression range:

2019-09-22T21:52:44: INFO : Narrowed inbound regression window from [b742509f, 284c8622] (3 builds) to [bcaada0f, 284c8622] (2 builds) (~1 steps left)
2019-09-22T21:52:44: DEBUG : Starting merge handling...
2019-09-22T21:52:44: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=284c8622607184580020cfa5f08e59677862f08d&full=1
2019-09-22T21:52:46: DEBUG : Found commit message:
Bug 1550030 - Show breakpoint icon in inspector for DOM Mutation Breakpoints r=gl

Differential Revision: https://phabricator.services.mozilla.com/D45036

2019-09-22T21:52:46: DEBUG : Did not find a branch, checking all integration branches
2019-09-22T21:52:46: INFO : The bisection is done.
2019-09-22T21:52:46: INFO : Stopped

Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Regressed by: 1550030

Thanks for filing, confirming the bug.

Discussed this with :fvsch, who suggested fixing this by adding box-sizing: border-box to

/* Force height and width (possibly overflowing) from inline elements.
 * This allows long overflows of text or input fields to still be styled with
 * the container, rather than the background disappearing when scrolling */
#root {
  float: left;
  min-width: 100%;
  /* Accommodates for DOM mutation breakpoint and pseudo icons */
  padding-left: 5px;
}

https://searchfox.org/mozilla-central/rev/45f30e1d19bde27bf07e47a0a5dd0962dd27ba18/devtools/client/themes/markup.css#43-44
(the padding-left: 5px is responsible for the regression here)

Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2

Yup, on the #root selector.

Another option is to remove those two lines:
https://searchfox.org/mozilla-central/source/devtools/client/themes/markup.css#43-44

And to reimplement this increased padding by adding it to the .child selector which currently has this style:

.child {
  margin-left: -1000em;
  padding-left: 1001em;
}

Since our font-size is 11px, that resolves to -11000px of margin-left and 11011px of padding-left, hence a "visible" padding of 11px, which we want to increase to 16px (+5px). So we could have:

.child {
  margin-left: -10000px;
  padding-left: 10016px;
}

(The supper big negative margin and padding are a trick used to manage the selection backgrounds and indent guides.)

Assignee: nobody → florens
Status: NEW → ASSIGNED
  • Add box-sizing:border-box to root container to avoid horizontal scrollbar.
  • Control indent size from CSS only, using calc() and a custom property for the indent count.
  • Set indent size explicitly to 12px instead of 1em (11px): even number and matches Chrome.
  • Remove padding-left from tag lines, so that left padding is a combination of root padding + indents only.
Attachment #9095285 - Attachment description: Bug 1583034 - Improve markup breakpoint position and left padding; r=davidwalsh → Bug 1583034 - Improve markup breakpoint position and left padding; r=gl

@fvsch, I am wondering if this needs any ux-review/feedback from victoria

Flags: needinfo?(florens)

In particular, regarding the indent sizing changing to 12px.

Hi Victoria, I'm considering tweaking our current indent size from 11px to 12px.
Mostly because I like even numbers ^^, and it turns out Chrome uses 12px so why not match them since it's very close.

Here are screenshots at 10px (smaller), 11px (current) and 12px (bigger).
Any preference?

Flags: needinfo?(florens) → needinfo?(victoria)

I know that visually, it's a minor thing, but I'd like to keep 11px for a narrower gutter (it's just wide enough to fit the new DOM breakpoints design, so 10px would be a bit too tight).

Flags: needinfo?(victoria)

Keeping a 11px indent plus a 6px extra space for the breakpoints.
Total on the first level = 17px, which matches the design exactly.

Attachment #9095285 - Attachment description: Bug 1583034 - Improve markup breakpoint position and left padding; r=gl → Bug 1583034 - Fix markup view root padding and width; r=gl
Attachment #9095285 - Attachment description: Bug 1583034 - Fix markup view root padding and width; r=gl → Bug 1583034 - Fix markup view root padding and content width; r=gl
Pushed by florens@fvsch.com:
https://hg.mozilla.org/integration/autoland/rev/5384c6488093
Fix markup view root padding and content width; r=gl
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
QA Whiteboard: [qa-71b-p2]
You need to log in before you can comment on or make changes to this bug.