1455392 - Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement

Status: RESOLVED FIXED

(Core :: XUL, task)

Description

[Brian Grinstead [:bgrins]]

This is only ever used to check for CHECKSTATE_MIXED which in turn is used to set mozilla::a11y::states::MIXED

- https://searchfox.org/mozilla-central/search?q=%3A%3Acheckstate&path=

However, there's only one place we ever set that state - in the about:addons 'Recent Updates' list. The mixed state is set when the list is ascending, which doesn't seem right. The element is 'checked' whether it's ascending or descending. https://searchfox.org/mozilla-central/rev/59a9a86553e9bfd9277202748ff791fd9bc0713b/toolkit/mozapps/extensions/content/extensions.xml#302

If we get rid of that consumer, I believe we can remove all the idl and accessibility code handling this case, which should help unblock switching to an actual html:input (since the only remaining attribute on nsIDOMXULCheckboxElement.idl after this and Bug 1455359 will be 'checked').

Comment 1

[Brian Grinstead [:bgrins]]

Andrew - we could update the checkState to always be '1' for the selected header. But.. could we just remove this UI? The default sort order is by Date (most recent first) which seems like the correct order for a 'recent updates' list.

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1455392 - Remove sortable UI in about:addons recent updates;

This UI is the only consumer of `checkState`, and it's using it in a
confusing way represent a ascending and descending sorts on the checkbox.
And also since the default sort for Recent Updates (most recent first)
is reasonable, go ahead and remove the binding altogether.

Review commit: https://reviewboard.mozilla.org/r/238162/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/238162/

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: Bug 1455392 - Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement

There aren't any consumers for the third state, so `checked` is enough.

Review commit: https://reviewboard.mozilla.org/r/238164/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/238164/

Comment 4

[alexander :surkov (:asurkov)]

Comment on attachment 8969410 [details]
Bug 1455392 - Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement

https://reviewboard.mozilla.org/r/238164/#review243884

r=me for a11y part

Comment 5

[Andrew Swan [:aswan]]

(In reply to Brian Grinstead [:bgrins] from comment #1)
> Andrew - we could update the checkState to always be '1' for the selected
> header. But.. could we just remove this UI? The default sort order is by
> Date (most recent first) which seems like the correct order for a 'recent
> updates' list.

Works for me, ccing Emanuela and Markus from UX so they are not surprised.
Odds are high that somebody will pop up in the next few weeks and say that they have a hundred extensions installed and now they can't effectively find things in recent updates, but I don't think that should stand in the way of de-XBLing

Comment 6

[Andrew Swan [:aswan]]

Comment on attachment 8969409 [details]
Bug 1455392 - Remove sortable UI in about:addons recent updates;

https://reviewboard.mozilla.org/r/238162/#review243886

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: sort-headers.png

For reference - this is the UI we are talking about

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8969409 [details]
Bug 1455392 - Remove sortable UI in about:addons recent updates;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/238162/diff/1-2/

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8969410 [details]
Bug 1455392 - Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/238164/diff/1-2/

Comment 10

[Brian Grinstead [:bgrins]]

Comment on attachment 8969410 [details]
Bug 1455392 - Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement

Requesting review for the dom idl changes

Comment 11

[Brian Grinstead [:bgrins]]

Comment on attachment 8969409 [details]
Bug 1455392 - Remove sortable UI in about:addons recent updates;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/238162/diff/2-3/

Comment 12

[Brian Grinstead [:bgrins]]

Comment on attachment 8969410 [details]
Bug 1455392 - Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/238164/diff/2-3/

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8969410 [details]
Bug 1455392 - Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement

https://reviewboard.mozilla.org/r/238164/#review244262

r=me, but you don't have to rev iids...

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a24709c044b
Remove sortable UI in about:addons recent updates;r=aswan
https://hg.mozilla.org/integration/autoland/rev/e166407fe8b0
Remove checkState from nsIDOMXULCheckboxElement and nsIDOMXULButtonElement;r=bz,surkov

Comment 15

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/8a24709c044b
https://hg.mozilla.org/mozilla-central/rev/e166407fe8b0

Comment 16

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336