Closed Bug 1661120 Opened 1 year ago Closed 1 year ago

Modal print dialog overlaps devtools bottom-bar/sidebar in a weird way (with a thin line at devtools' edge)

Categories

(Toolkit :: Printing, defect, P2)

defect

Tracking

()

VERIFIED FIXED
82 Branch
Tracking Status
firefox81 --- verified
firefox82 --- verified

People

(Reporter: dholbert, Assigned: mtigley)

References

(Blocks 1 open bug)

Details

(Whiteboard: [print2020_v81])

Attachments

(2 files)

Attached image screenshot

STR:

  1. Ensure you have print.tab_modal.enabled = true in about:config
  2. While viewing some printable website (e.g. this page), do Ctrl+Shift+k to open devtools.
    (Be sure DevTools is docked to the bottom; this isn't strictly necessary but it's a bit easier to see the issue this way.)
  3. Resize devtools or your window so that devtools occupies ~half of your window height (to ensure that there will be overlap in the next step).
  4. Ctrl+P to open modal print dialog,

ACTUAL RESULTS:
At the top of the area where devtools and the print dialog overlap, there's a thin line going across the print dialog (a visual artifact of some sort).

EXPECTED RESULTS:
No such line.

Severity: -- → S4
Priority: -- → P2
Whiteboard: [print2020] → [print2020_v81]

Setting z-index: 2 on the .dialogStack in browser.css seems to do the trick. This makes the modal print dialog have a higher stacking order than the one set for splitter.devtools-horizontal-splitter in https://searchfox.org/mozilla-central/source/devtools/client/themes/splitters.css#61.

I haven't observed UX regressions trying this out.

Assignee: nobody → mtigley
Status: NEW → ASSIGNED
Pushed by mtigley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ef2f4122bb76
Set a higher z-index value on the dialogStack. r=dao
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Can you request Beta approval?

Flags: needinfo?(mtigley)

Comment on attachment 9172042 [details]
Bug 1661120 - Set a higher z-index value on the dialogStack. r=Gijs

Beta/Release Uplift Approval Request

  • User impact if declined: The DevTools splitter element will overlap the modal print preview UI when DevTools is opened, which can be jarring and look unpolished to the user.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): A small and trivial change to the modal print preview's styling, so the change shouldn't be risky.
  • String changes made/needed:
Flags: needinfo?(mtigley)
Attachment #9172042 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Comment on attachment 9172042 [details]
Bug 1661120 - Set a higher z-index value on the dialogStack. r=Gijs

approved for 81.0b3

Attachment #9172042 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Are there other dialogs beyond print preview that use dialogStack and could be impacted?

Flags: needinfo?(mtigley)

I'm not aware of others, but I think Gijs or pbz would know better. I'll transfer the needinfo to them.

Flags: needinfo?(mtigley) → needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(pbz)

There are some, like the one we use to warn users about submitting forms from a secure (https) page to an insecure (http) page, but those are much less tall and so are less likely to be impacted. I don't think that on 81 beta there's any other dialogs we should be concerned about, but I'll leave Paul to confirm that.

Flags: needinfo?(gijskruitbosch+bugs)

Thanks for the fix and for checking in with me. There should be no other consumers in 81 beta.

Flags: needinfo?(pbz)

Confirmed as verified fixed on MacOS 10.14 and Windows 10 x64bit with latest Nightly 82.0a1 (20200827093043).

Also confirmed as verified fixed on MacOS 10.14 and Windows 10 x64bit with latest Beta 81.0b3 (20200827203325).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.