Closed Bug 1419170 Opened 2 years ago Closed 2 years ago

Remove the statusbar and statusbarpanel bindings from m-c

Categories

(Toolkit :: XUL Widgets, task, P5)

task

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

(Blocks 1 open bug)

Details

(Whiteboard: [xbl-remove-unused])

Attachments

(1 file)

Once these aren't used in view source windows anymore, the bindings and associated CSS / tests can be removed from m-c. They are still used in comm-central, so will need to be ported there.

Note that there's a bunch of accessibility and widget code that I believe could be removed from m-c as well, but that can be tracked in separate bugs if we want to do so. If we key accessibility off the tag name instead of XBL role then accessibility tests should still pass even if the bindings are not in m-c.

Some relevant searches:

https://dxr.mozilla.org/mozilla-central/search?q=statusbar&redirect=false
https://dxr.mozilla.org/mozilla-central/search?q=statusbarpanel&redirect=true
https://dxr.mozilla.org/mozilla-central/search?q=chromeclass-status&redirect=true
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Depends on: 1419179
Brian, do you plan to remove the statusbar and statusbarpanel styles in global.css too?
(In reply to Richard Marti (:Paenglab) from comment #2)
> Brian, do you plan to remove the statusbar and statusbarpanel styles in
> global.css too?

Yeah I think anything targeting 'statusbar', 'statusbarpanel', or '.statusbar-resizerpanel' should probably move to comm-central since the binding won't be in toolkit anymore. I noticed there's at least one example of `-moz-appearance: statusbar` for a non-statusbar tag that would stay though (https://searchfox.org/mozilla-central/source/toolkit/themes/osx/mozapps/update/updates.css#37).
I think we should keep the `window[chromehidden~="status"] .chromeclass-status` rule around though [0], unless/until the platform support for dom.disable_window_open_feature.status was removed (which I'd suggest is done in a separate bug from this one, outside of the XBL replacement work)

[0]: https://searchfox.org/mozilla-central/source/toolkit/content/xul.css#37
Paolo, we'll need to decide what state statusbar should be left in. Here's what would happen if we go with this patch, in (besides the bindings being removed):

a) <statusbar> still gets accessibility. This prevents accessibility breakage in comm-central and is a trivial change now that we have xulmap.h
b) We still have the `dom.disable_window_open_feature.status` feature and associated CSS (see Comment 4)
c) statusbar widget code still exists, as there's at least one place using `-moz-appearance: statusbar` in m-c (see Comment 3)

IMO there isn't a big benefit to completely tearing out (a), (b), and (c) as part of this work, but let me know if you feel otherwise.
Depends on: 1418094
Priority: -- → P5
(In reply to Brian Grinstead [:bgrins] from comment #6)
> IMO there isn't a big benefit to completely tearing out (a), (b), and (c) as
> part of this work

I agree, I wouldn't consider any of these issues a part of the "remove XBL" work. It's still worth filing individual bugs for them, and start a backlog of "remove XUL" bugs if we haven't one already. We'll probably find more bugs as we go. We still have to decide a strategy for removing XUL elements, and I can see the remaining styles and widgets either being removed individually in the medium run, or being mass-migrated to comm-central in a more distant future.
Comment on attachment 8930611 [details]
Bug 1419170 - Remove the statusbar and statusbarpanel bindings;

https://reviewboard.mozilla.org/r/201744/#review207816
Attachment #8930611 - Flags: review?(paolo.mozmail) → review+
The patch doesn't apply anymore:
patching file accessible/base/nsAccessibilityService.cpp
Hunk #1 FAILED at 269
(In reply to :aceman from comment #10)
> The patch doesn't apply anymore:
> patching file accessible/base/nsAccessibilityService.cpp
> Hunk #1 FAILED at 269

Rebased and pushed back to mozreview. From https://bugzilla.mozilla.org/show_bug.cgi?id=1418094#c12 it looks like this is ready to go from the comm-central side so if try looks good I'll land it
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/38f11a0d4111
Remove the statusbar and statusbarpanel bindings;r=Paolo
https://hg.mozilla.org/mozilla-central/rev/38f11a0d4111
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
See Also: → 1424164
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.