1590180 - Remove usage of `display: -moz-groupbox` and `-moz-appearance: groupbox`

Status: RESOLVED FIXED

(Toolkit :: Printing, task)

Description

[Tim Nguyen :ntim]

Only one usage in mozilla-central: https://searchfox.org/mozilla-central/search?q=display%3A+-moz-groupbox&case=false&regexp=false&path=

Probably not worth keeping the whole nsGroupBoxFrame implementation around just for it.

Comment 1

[Brian Grinstead [:bgrins]]

What does this layout do? Wondering if it could be replaced with a box with some styling or if there's something more involved.

Comment 2

[Mats Palmgren (inactive)]

It appears to be some kind of fieldset-like layout for XUL:
https://developer.mozilla.org/en-US/docs/Archive/Mozilla/XUL/groupbox
Maybe the markup can use HTML <fieldset>/<legend> instead? (assuming it's a design we want to keep)

Comment 3

[Mats Palmgren (inactive)]

Looking at the source, I don't see anything special other than the painting of the border:
https://searchfox.org/mozilla-central/source/layout/xul/nsGroupBoxFrame.cpp
It would be great if we could get rid of this code.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Mats Palmgren (:mats) from comment #3)

Looking at the source, I don't see anything special other than the painting of the border:
https://searchfox.org/mozilla-central/source/layout/xul/nsGroupBoxFrame.cpp
It would be great if we could get rid of this code.

It seems pretty doable to update the windows printPageSetup page to not use it. What do you think we should do with the tests at https://searchfox.org/mozilla-central/search?q=display%3A+-moz-groupbox&case=false&regexp=false&path=?

Comment 5

[Mats Palmgren (inactive)]

The crashtests might have some value beyond -moz-groupbox, so replacing -moz-groupbox with -moz-box in those seems reasonable. The -moz-groupbox part of the mochitest should be removed.

Comment 6

[Mats Palmgren (inactive)]

Note that there's also a StyleAppearance::Groupbox (-moz-appearance:groupbox) and some a11y stuff related to this display value:
https://searchfox.org/mozilla-central/search?q=groupbox&path=
I'm guessing those can be removed too.

Comment 7

[Mats Palmgren (inactive)]

It appears there are some <groupbox> elements in Preferences etc though, which I assume creates a XULGroupboxAccessible, so let's keep the a11y stuff as is for now. (we can address that after it's been replaced with proper HTML some time in the future)

The -moz-appearance value should be possible to remove now though as printPageSetup.css appears to be the only user:
https://searchfox.org/mozilla-central/search?q=appearance%3A+groupbox&path=
(this might be better to do as a separate bug though, in the interest of keeping the patch size smaller and more targeted)

Comment 8

[Dave Townsend [:mossop]]

Attachment: Bug 1590180: Remove use of `display: -moz-groupbox` and `-moz-appearance: groupbox` from the page setup dialog. r=dao

The display property appears to do nothing here and the appearance closely
matches that of a fieldset so switching to that seems to work fine.

Comment 9

[Dave Townsend [:mossop]]

Attachment: Screenshot

This is what it looks like with the change.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/018aa28cf32f
Remove use of `display: -moz-groupbox` and `-moz-appearance: groupbox` from the page setup dialog. r=dao

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/018aa28cf32f

Comment 12

[Tim Nguyen :ntim]

Splitting the platform removal off to a new bug, because different patches in different releases on the same bug can get confusing.