349251 - "l" key "grabbed" by the software update dialog (without the "alt"), results in showing me an empty list of extensions that are not compatible

Status: RESOLVED WONTFIX

(Toolkit :: Application Update, defect)

Description

[(not reading, please use [e-mail] instead)]

my key stroke (in this case "l") is being "grabbed" by the software update dialog and results in showing me an empty list of extensions that are not compatible.

note, I am typing "l", not "alt l".

to reproduce, you will need to set a few prefs (to force a software update prompt) and then all you have to do is hit "l". 

I'll list the prefs I have set to make this easy to reproduce.

in my case, my list of incompatible extensions are empty, so the dialog is even more suprising, since I don't even have the "Show _L_ist" ui (it is hidden).
  
I was concerned this was a regression from something I'm working on, but I see it in ff2b1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060710 Firefox/2.0b1) and the trunk.

"d" has the same problem "_D_ownload & Install Now >>".  

Interestingly, "a" (L_a_ter) does not.

Comment 1

[(not reading, please use [e-mail] instead)]

obviously, "d" is scary, as it could lead to getting an update when you really do have extension or theme incompatibilities.

Comment 2

[(not reading, please use [e-mail] instead)]

I don't think bug #334808 introduced this, but I'll compare to a build before that change.

Comment 3

[(not reading, please use [e-mail] instead)]

Attachment: screen shot

Comment 4

[(not reading, please use [e-mail] instead)]

in showIncompatibleItems(), in updates.js, I put:

    dump(new Error().stack);

and I see that:

Error()@:0
()@chrome://mozapps/content/update/updates.js:790
oncommand([object XULCommandEvent])@chrome://mozapps/content/update/updates.xul:
1
click()@:0
fireAccessKeyButton([object XULElement],"l")@chrome://global/content/bindings/bu
tton.xml:110
onxblkeypress([object KeyboardEvent])@chrome://global/content/bindings/button.xm
l:175
@:0

in updates.js, when this page (the "updates available" page) is shown, we focus the "next" button,see http://lxr.mozilla.org/mozilla1.8/source/toolkit/mozapps/update/content/updates.js#664

in this case, the "next" button is the "download" button, which explains why "d" without the alt key works.

it appears that when a button has focused, you can use the accesskey without the alt modifier.  (I never new that, neither did jeff walden, so I don't feel too bad.)

looking at http://lxr.mozilla.org/mozilla1.8/source/toolkit/content/widgets/button.xml#107, the access key "l" bug could be fixed if I disabled or hid the button, as well as the box, see http://lxr.mozilla.org/mozilla1.8/source/toolkit/mozapps/update/content/updates.xul#105

since the update dialog popups, is making the "next" button focused a great idea?  I could be typing, and the update dialog popups up, steals my focus, and I accidentally hit "d", and start downloading, even if I have incompatible extensions.

Comment 5

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

(In reply to comment #4)
> since the update dialog popups, is making the "next" button focused a great
> idea?  I could be typing, and the update dialog popups up, steals my focus, and
> I accidentally hit "d", and start downloading, even if I have incompatible
> extensions.

See also bug 311288, and bug 323041.

Comment 6

[(not reading, please use [e-mail] instead)]

thanks gavin.

questions:

1)  maybe we could add the the delay for the "updates available page"?
2)  I'm still not sure why when a button has focus the "List" button access key works with out alt.

but, I have a work around, which is to hide the button as well as the box.

Comment 7

[(not reading, please use [e-mail] instead)]

to updates.xul:

             <hbox pack="end">
               <button label="&listIncompatible.label;" accesskey="&listIncompat
ible.accesskey;"
+                      id="incompatibleButton" hidden="true"
                       oncommand="gUpdatesAvailablePage.showIncompatibleItems();
"/>

to updates.js:

       var incompatibleWarning = document.getElementById("incompatibleWarning");
       incompatibleWarning.hidden = false;
+      // fix for bug #349251
+      // the list incompatible button is also hidden by default
+      // so that the accesskey doesn't work
+      var incompatibleButton = document.getElementById("incompatibleWarning");
+      incompatibleWarning.hidden = false;

Comment 8

[Robert Strong (they/them - no direct email)]

see bug 347585 which would remove the remove the issues with popping up the dialogs for app / add-on updates and others as needed.

Comment 10

[(not reading, please use [e-mail] instead)]

sorry for the bug spam, re-assigning bugs back to default owner if I'm not working actively on them.

Comment 11

[(not reading, please use [e-mail] instead)]

the recent fix for bug #143065 doesn't apply here, but perhaps neil or mats have ideas?

Comment 12

[Mats Palmgren (inactive)]

(In reply to comment #6)
> 2)  I'm still not sure why when a button has focus the "List" button
> access key works with out alt.

AFAICT, this is the default native behaviour on Windows XP.

We have implemented this behaviour here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/button.xml&rev=1.21&root=/cvsroot&mark=96-116,167-194#96

FWIW, removing the marked code blocks "fixes it" (and Alt+key still works).

The native behaviour on Linux (at least under KDE) is to require Alt,
so I'm not sure if it should be considered a bug there perhaps.
However, I think we strive to have reasonably compatible keyboard
behaviour between platforms where it makes sense.

So, in general, this is not a bug, it's a feature.
If we want to do something to avoid it in this particular case is
a different question...

I think the solution is to simply stop popping up dialogs unless
they are requested by the user.

Comment 13

[[e-mail]]

(In reply to comment #12)
>AFAICT, this is the default native behaviour on Windows XP.
Actually Windows also does this for checkboxes and radiobuttons, and in fact Windows also triggers hidden buttons. That said, maybe we should strengthen the check we do for hidden buttons - if the button has zero width or height then it's probably not one we want to trigger.

Comment 14

[Mats Palmgren (inactive)]

(In reply to comment #13)
> Actually Windows also does this for checkboxes and radiobuttons [...]
> maybe we should strengthen the check we do for hidden buttons

The normal Alt+key event handling in ESM does that, so we should
probably just remove this code and make ESM deal with it.
I filed bug 406933 on that, since the original problem in this bug
is more about focus stealing.

Comment 15

[Robert Strong (they/them - no direct email)]

This ui was removed by the landing of bug 324121... resolving -> wontfix but if you prefer a different resolution feel free to change it.