313205 - software update "Check Now" uses the same accesskey as "Next"

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Reuven Gonen]

when Firefox or Thunderbird checks for updates for an extension, the "Check Now"
and "Install Now" buttons use the same accesskey as the wizard "Next" button. in
english it looks ok, since all of these words have "N" in them, but can be a
problem in other locales.

Comment 1

[Reuven Gonen]

Attachment: screenshot of problem

this screenshot shows the problem on the "he" and "fr" locales

Comment 2

[Scott MacGregor]

accepting a patch for this would have significant impact for all of the
localizers just days away from 1.5 being done. I'll let Axel comment as to
whether he would approve such a change.

Comment 3

[Asaf Romano (gone)]

Er, it seems the entities are already there, they're not set right though.

Comment 4

[Asaf Romano (gone)]

Attachment: patch

Reuven, does this help?

Comment 5

[Reuven Gonen]

(In reply to comment #2)
> accepting a patch for this would have significant impact for all of the
> localizers just days away from 1.5 being done. I'll let Axel comment as to
> whether he would approve such a change.

This problem seems to be only on the trunk since the wizard 'Next' and 'Back'
access keys(added by Bug 221783) were not added to the branch. in the branch
these buttons simply don't have any accesskeys.

Comment 6

[Reuven Gonen]

(In reply to comment #4)
> Created an attachment (id=200310) [edit]
> patch
> 
> Reuven, does this help?

Yes, it works. Thanks!

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 200310 [details] [diff] [review]
patch

this shouldn't hurt l10n since the strings are already there.

Comment 8

[Asa Dotzler [:asa]]

sounds like we don't have this problem on the branch because we lack access keys
on the branch. If that's not correct, let's get this re-nominated. Thanks.

Comment 9

[Reuven Gonen]

(In reply to comment #8)
> sounds like we don't have this problem on the branch because we lack access keys
> on the branch. If that's not correct, let's get this re-nominated. Thanks.

actually the problem is that the access keys in update.properties are not used,
and this problem is on the branch as well. curently the "don't check" and the
"Cancel" button don't have access keys even in the english version.

see:
http://lxr.mozilla.org/seamonkey/source/toolkit/locales/en-US/chrome/mozapps/extensions/update.properties

Comment 10

[Asaf Romano (gone)]

Renominating, see comment 9.

Comment 11

[Asaf Romano (gone)]

Checking in update.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/update.js,v  <--  update.js
new revision: 1.21; previous revision: 1.20
done

Comment 12

[Asaf Romano (gone)]

Comment on attachment 200310 [details] [diff] [review]
patch

I only check this in on trunk, Reuven.

Comment 13

[Asaf Romano (gone)]

checked*

Comment 14

[Asa Dotzler [:asa]]

Mano, can you explain what this patch is doing? If we're currently not using the access keys on the branch, why is this necessary?

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 200310 [details] [diff] [review]
patch

too late to work out adding access keys to our wizard.

Comment 16

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

(In reply to comment #15)
> (From update of attachment 200310 [details] [diff] [review] [edit])
> too late to work out adding access keys to our wizard.

Asa, this doesn't add accesskeys, it just makes it so that the existing accesskeys are applied correctly.

Comment 17

[Asa Dotzler [:asa]]

This isn't a problem on the branch because we don't have access keys enabled anywhere else in wizards on the branch at all. We're not even in agreement that we want access keys in wizards at all. The summary of this bug, "software update "Check Now" uses the same accesskey as "Next"" just doesn't matter since they're not actually exposed. That makes this a request to enable these access specific access keys and we're not going to do that.

Comment 18

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

Comment on attachment 200310 [details] [diff] [review]
patch

This is needed on the branch now that bug 221783 was landed on the branch.

Comment 19

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

Comment on attachment 200310 [details] [diff] [review]
patch

This is the right thing to do though we shouldn't need the try catch if the button being set exists and since the same code is setting it it should always exist (e.g. it could cover up other bugs). I'll try to fix that and a few other issues with this wizard in another bug for 2.0

Comment 20

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

mozilla/toolkit/mozapps/extensions/content/update.js; new revision: 1.17.2.4;

Comment 21

[Alex Ihrig [:Thunderbird_Mail_DE]]

Bug is verified fixed in todays 1.8_Branch nightlies.

Comment 22

[Alex Ihrig [:Thunderbird_Mail_DE]]

*** Bug 329333 has been marked as a duplicate of this bug. ***