424317 - Add-on Updates dialogue shown at startup has buttons in the wrong order/position

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Grey Nicholson (they/them)]

Attachment: The offending dialogue

When there is an update available for an add-on, Firefox advertises this at startup by showing a dialogue.

The buttons in this dialogue are arranged:
[Install Updates] [Show Information]          [Skip]

On Gnome, this should be:
[Show Information]          [Skip] [Install Updates]

because the default, confirming action is always at the far bottom-right.

Comment 1

[Madhava Enros [:madhava]]

Agreed - skip and install should be the cancel and OK equivalents at the bottom right, with show information at the left.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Not a blocker, but hopefully a quick fix is possible?

Comment 3

[Nochum Sossonko [:Natch]]

If someone could tell me where to find this (under toolkit/extensions?) I'd be glad to give it a try.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #3)
> If someone could tell me where to find this (under toolkit/extensions?)

A good way to find out where a dialog like that is implemented is to search for one of the strings, e.g.:
http://mxr.mozilla.org/mozilla-central/search?string=new+updates+available&find=toolkit
which leads to:
http://mxr.mozilla.org/mozilla-central/search?string=newUpdatesAvailableMsg
which leads to:
http://hg.mozilla.org/mozilla-central/index.cgi/annotate/43e89bd626d2/toolkit/mozapps/extensions/content/extensions.js#l1121

It's actually using a specific "view" of the normal addons manager, so this might be a bit of a pain to fix (since the buttons are shared across views).

Comment 5

[Nochum Sossonko [:Natch]]

Attachment: Would this work?-untested

not setting review flags yet b/c i have no way of testing this, found similar code in other toolkit apps, is this the way to go (btw in non-gnome apps it displays fine, i.e. no different then it used to)?

Comment 6

[Nochum Sossonko [:Natch]]

Comment on attachment 335670 [details] [diff] [review]
Would this work?-untested

still more work to do, e.g. the default button is still the old one even on gnome, easy to implement just wanted to get some feedback on this approach.

Comment 7

[Dave Townsend [:mossop]]

Ah yes I hadn't quite realised this was going to be platform specific. I guess a good way to start here would be to get feedback from the UX team on the correct ordering for all the buttons. Just come up with a suggestion for how you think the buttons should be arranged for the platforms. Once UX are happy I suspect the patch would be simplest done by just changing extensions.xul, commandBarBottom can be ordered differently on the different platform as necessary.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

The correct button ordering per platform has already been settled in our dialog implementation, you can base the button ordering on that once you map the available actions to the "accept", "cancel", "extra1" and "extra2" semantics (perhaps you already have). You should be able to use the same ifdef there too (XP_UNIX instead of XP_GNOME).

http://hg.mozilla.org/mozilla-central/index.cgi/annotate/6abe7e4a2e59/toolkit/content/widgets/dialog.xml#l20

Comment 9

[Nochum Sossonko [:Natch]]

Attachment: According to dialog.xml

* Uses XP_UNIX instead of XP_GNOME (I guess MACOSX doesn't also define XP_GNOME, or that the layout is the same for both).

* Mapped according (as best I could) to dialog.xml

disclosure == installFileButton
help == showUpdateInfoButton
extra (there's only one, I think) == checkUpdatesAllButton
spacer
cancel == skipDialogButton
accept == installUpdatesAllButton

The rest of the buttons are toggled when the above ones are hidden, so they remain in their respective places (the only one that's maybe a question is continueDialogButton, but I think it's in the right place, i.e. the right side of the bar).

Comment 10

[Nochum Sossonko [:Natch]]

Attachment: Sorry this is really it!

Messed around with the file so much I lost my bearings!
Here's the intended file.

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 335805 [details] [diff] [review]
Sorry this is really it!

I disagree with the mapping you have come up with for the buttons, but even then the patch you propose does not produce the mapping you suggest.

Here is the mapping that I think is correct:

accept = continue, installUpdateAll
cancel = skip
extra1 = checkUpdatesAll
extra2 = installFile
disclosure = showUpdateInfo, hideUpdateInfo, getMore

Let's see if that makes sense in all the views.

Comment 12

[Nochum Sossonko [:Natch]]

The reason I only changed the accept buttons is because the implementation of the buttons in extensions.xul doesn't follow anything in dialog.xml, including your definition, so I just changed what this bug was asking for and figured that might work.

Now that you say you want it all changed, should I change the regular (i.e. non-GNOME) layout as well? Either way would basically mean ifdefing _all_ the buttons for both platforms.

Comment 13

[Nochum Sossonko [:Natch]]

Attachment: For conformance with dialog.xml

Here is a patch for both layouts according to the mapping from comment 11.

Comment 14

[Dave Townsend [:mossop]]

Comment on attachment 336684 [details] [diff] [review]
For conformance with dialog.xml

I think mapping them all is the right way to go, it should hopefully make future decisions about where to put buttons simpler than trying to deal with the existing configurations

Comment 15

[Dave Townsend [:mossop]]

Landed, thanks for the patch.

http://hg.mozilla.org/mozilla-central/rev/72df909fdc15