Remove access key in add-on update notifications

RESOLVED FIXED in Firefox 57

Status

()

Firefox for Android
Add-on Manager
P2
normal
RESOLVED FIXED
a month ago
a month ago

People

(Reporter: flod, Assigned: aswan)

Tracking

unspecified
Firefox 57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a month ago
This happened twice already: code for Webextensions is ported from browser, and with that we port access keys. 
https://hg.mozilla.org/mozilla-central/diff/314cd9bfe986/mobile/android/locales/en-US/chrome/browser.properties#l1.17

Access keys in Android don't make any sense and they should not be ported. We should make a note in review to check for this mistake.

Can we get a fix in quickly?
(Reporter)

Comment 1

a month ago
I'm not sure if I'm misreading the code, but is the button even used?
https://hg.mozilla.org/mozilla-central/rev/314cd9bfe986
Flags: needinfo?(topwu.tw)
Flags: needinfo?(aswan)

Comment 2

a month ago
Sorry I'm not familiar with this Javascript part in Fennec, I should only review the first three parts in bug 1391579 and leave the last one to :sebastian or other experts.

:aswan would you mind take a look at this issue? Thank you.
Flags: needinfo?(topwu.tw)
(Reporter)

Updated

a month ago
Flags: needinfo?(s.kaspari)
(Assignee)

Comment 3

a month ago
Sorry, this was my fault.  I wrote this patch quite a while before the "remove accessKeys on android" bug and fix went in, and I overlooked it when landing.

(In reply to Francesco Lodolo [:flod] from comment #1)
> I'm not sure if I'm misreading the code, but is the button even used?

Yes, but the process to get it to show up manually is pretty tedious.  You need to install a webextension, then make an update available for that extension that requires one or moer new (promptable) permissions, then wait for the browser to check for updates (or use the debugger or preferences to trigger a quicker check).  Then the extension's entry in about:addons gets an "Update" button and when that is pressed, the permissions dialog shows up with "Update" as one of the options.

In any case, I'll put up the patch to remove the access key right now, sorry again for the hassle.
Flags: needinfo?(s.kaspari)
Flags: needinfo?(aswan)
Comment hidden (mozreview-request)
(Assignee)

Updated

a month ago
Attachment #8908693 - Flags: review?(francesco.lodolo)
(Reporter)

Comment 5

a month ago
mozreview-review
Comment on attachment 8908693 [details]
Bug 1400129 Remove unused accessKey l10n property

https://reviewboard.mozilla.org/r/180342/#review185500

Thanks for taking the time to explain the code ;-)
Attachment #8908693 - Flags: review?(francesco.lodolo) → review+
(Reporter)

Updated

a month ago
Assignee: nobody → aswan

Comment 6

a month ago
Pushed by francesco.lodolo@mozillaitalia.org:
https://hg.mozilla.org/integration/autoland/rev/18dad63ef0c6
Remove unused accessKey l10n property r=flod
https://hg.mozilla.org/mozilla-central/rev/18dad63ef0c6
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.