Bug 1520625 (burn-xul-grids)

[meta] Remove XUL grid layout usage

NEW
Unassigned

Status

()

2 months ago
a month ago

People

(Reporter: ntim, Unassigned)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug, {meta})

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

2 months ago

All the grid layouts:

browser/base/content/pageinfo/pageInfo.xul
browser/base/content/sanitize.xul
browser/components/preferences/languages.xul
browser/components/preferences/connection.xul
browser/components/preferences/sanitize.xul
browser/components/preferences/browserLanguages.xul
browser/components/preferences/fonts.xul
browser/components/preferences/in-content/main.xul
browser/components/preferences/in-content/privacy.xul
security/manager/pki/resources/content/device_manager.xul
security/manager/pki/resources/content/changepassword.xul
security/manager/pki/resources/content/certViewer.xul
security/manager/pki/resources/content/downloadcert.xul
security/manager/pki/resources/content/setp12password.xul
toolkit/components/printing/content/printPageSetup.xul
toolkit/components/printing/content/printPreviewProgress.xul
toolkit/components/printing/content/printProgress.xul
toolkit/components/prompts/content/commonDialog.xul
toolkit/components/prompts/content/tabprompts.jsm
toolkit/mozapps/extensions/content/pluginPrefs.xul
toolkit/mozapps/extensions/content/extensions.xul
toolkit/mozapps/preferences/changemp.xul
toolkit/mozapps/update/content/updates.xml

...and tests+examples

(Reporter)

Updated

2 months ago
Alias: burn-xul-grids
(Reporter)

Updated

2 months ago
Depends on: 1520630

A couple of notes from chat:

Most of the instances actually are grids, and could be replaced with HTML tables or CSS grid
However, one instance has fixed-width column that I converted to use XUL flex layout

I'm curious - could we change the UA styles on the XUL <columns>, <rows>, <column> and <row> elements to use CSS grid display and make this work without having to rewrite as many consumers?

(Reporter)

Updated

2 months ago
Depends on: 1520661
Keywords: meta
(Reporter)

Updated

2 months ago
Depends on: 1521285
(Reporter)

Updated

2 months ago
Depends on: 1521287
(Reporter)

Updated

2 months ago
Depends on: 1521290
(Reporter)

Updated

2 months ago
Depends on: 1521294
(Reporter)

Updated

2 months ago
Depends on: 1521295
(Reporter)

Updated

2 months ago
Depends on: 1521296
(Reporter)

Updated

2 months ago
Depends on: 1521298
Blocks: 1521483
(Reporter)

Updated

2 months ago
Depends on: 1524777
(Reporter)

Updated

2 months ago
Blocks: 1525737
(Reporter)

Updated

2 months ago
No longer blocks: 1521483
(Reporter)

Updated

2 months ago
Depends on: 1525739
(Reporter)

Updated

2 months ago
Depends on: 1525747

could we change the UA styles on the XUL <columns>, <rows>, <column> and <row> elements to use CSS grid display and make this work without having to rewrite as many consumers?

This is a pertinent question.

Even if it doesn't work in all details in all cases, it's still easier to fix only those details than change everything.

Updated

a month ago
Flags: needinfo?(ntim.bugs)
(Reporter)

Comment 3

a month ago

Hi Ben,

Just to make sure I answer all your questions (including the ones from bug 1527003):

  • Why are we removing XUL grids ?

To remove the underlying implementation given its extra maintenance cost and the presence of alternatives (CSS grid/HTML tables).

  • Could we change the UA styles on the elements without refactoring everything ?

This:

grid {
  display: table;
  table-layout: fixed;
  width: 100%;
}

grid row {
  display: table-row;
}

grid row > * {
  display: table-cell;
}

grid row > * moz-input-box {
  width: 100%;
}

more-or-less works in some grids (like the certificate viewer one), but might as well refactor to HTML tables than do this.

  • Why can't we use CSS grid ?

I've played with it myself and unfortunately, CSS grid does not work well with the display: -moz-box children. Note that using CSS grid requires a bit of refactoring and there's no CSS that can be applied to emulate the XUL grid elements.

  • What is the timing of the removal ?

Not planning to remove the implementation immediately. I'm just landing whatever can be landed without ugly hacks atm, meaning:

  1. For single-rowed/columned or fixed-dimension grid layouts (eg. one column has all children of the same width and the other column flexes), just converting to XUL flexbox works (done with most grids that have been removed so far, except the ones below)

  2. For places where HTML tables work and actually looks cleaner in terms of markup (bug 1524777, potentially some others ?)

  3. Just removing the grid if it provides no or not much benefit (bug 1520661, bug 1521296)

For the grids that don't fall into those 3 categories, they're likely going to be removed during/after flexbox emulation (bug 1033225) to be CSS grids (:bgrins might know about the timing of this).

TB doesn't have to remove everything right away, but it would be nice to look at the easy ones that fall in the previous categories and start thinking about the big pieces if there are any, especially given that TB has more usages (~120, eg. about 6 times more than m-c!) and different needs. Might not be nice to leave everything to the last minute ;)

Flags: needinfo?(ntim.bugs)
(Reporter)

Comment 4

a month ago

I've played with it myself and unfortunately, CSS grid does not work well with the display: -moz-box children. Note that using CSS grid requires a bit of refactoring and there's no CSS that can be applied to emulate the XUL grid elements.

Just to be clear, I've only tried CSS grid on browser/components/preferences/languages.xul, result was that the child richlistbox looked messed up (all items appeared on the same line, instead of separate lines). I haven't tried it on others. CSS grid might work perfectly well in other places. I chose to go with an HTML table in bug 1524777, because it actually simplified the markup and because CSS grid wouldn't be better for this specific usage, in terms of markup.

Tim, thanks for the answers.

Specifically, what I'm suggesting is to change toolkit/content/xul.css to include CSS rules that make the existing <grid> XUL elements function in most cases.

Special cases can still be adapted. But that would save a ton of work for re-implementing existing dialogs. We got 3 grids in a single dialog alone.

As for HTML in XUL (including HTML table), I've personally had very bad experiences when line wrap was required.

(Reporter)

Updated

a month ago
Depends on: 1529534
You need to log in before you can comment on or make changes to this bug.