Closed Bug 1583925 Opened 5 years ago Closed 5 years ago

Remove XUL grid from toolkit/components/prompts/content/commonDialog.xul

Categories

(Toolkit Graveyard :: Notifications and Alerts, task)

task
Not set
normal

Tracking

(firefox72 disabled, firefox73 fixed)

VERIFIED FIXED
mozilla72
Tracking Status
firefox72 --- disabled
firefox73 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

Details

Attachments

(4 files)

No description provided.
Attached image Screenshot (deleted) —

A subgrid-based approach seems pretty promising for this one, since we want to keep the row-based markup.

This makes use of CSS subgrid in order to preserve the row-based markup that is needed by JS to hide certain rows.

Assignee: nobody → ntim.bugs

Visiting the dialog from comment 1 shouldn't have any visual difference before and after this change (I've tested on macOS, but it would be nice if someone could test on Linux/Windows).

Flags: qe-verify+

Mats, is it ok to start using subgrid in the Firefox UI?

Flags: needinfo?(mats)
Attached patch commonDialog.css.diff (deleted) — Splinter Review

Sure, I don't see a problem with that. I'd like to note though that it should be quite rare to need that kind of complexity. In this case for example, it seems you could use display:contents instead, which results in a much simpler frame tree.

Flags: needinfo?(mats)
Attached patch v2, commonDialog.css.diff (deleted) — Splinter Review

This gets rid of #filler too.

Thanks for taking a look into this! Nice trick with display: contents and thanks for catching #filler too :)

Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/70b2e0edb9a1
Replace commonDialog.xul XUL grid with CSS grid. r=dao
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Regressions: 1599996

Verified as fixed on Windows 10, macOS 10.15 and Ubuntu 18.04 using Firefox 72.0a1 (20191129094247).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Regressions: 1603397
Regressions: 1606617
Regressions: 1606152
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: