Add access keys to disable/re-enable tracking protection doorhanger

VERIFIED FIXED in Firefox 35

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: mmc, Assigned: alexbardas)

Tracking

Trunk
Firefox 35
Points:
2
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Updated

5 years ago
Points: --- → 2
Flags: qe-verify+
Flags: firefox-backlog+

Updated

5 years ago
Component: DOM: Security → General
OS: Linux → All
Product: Core → Firefox
Hardware: x86_64 → All
Assignee

Updated

5 years ago
Assignee: nobody → abardas
Status: NEW → ASSIGNED
Iteration: --- → 35.1
Assignee

Comment 1

5 years ago
I've also fixed the access key for mixed content enable option, as seen in bug 1045809 comment 12 .
Attachment #8488288 - Flags: review?(jaws)

Updated

5 years ago
QA Contact: mwobensmith
Comment on attachment 8488288 [details] [diff] [review]
Add access keys for tracking protection doorhanger

Review of attachment 8488288 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +731,5 @@
>  <!ENTITY mixedContentBlocked2.options "Options">
>  <!ENTITY mixedContentBlocked2.unblock.label "Disable protection for now">
>  <!ENTITY mixedContentBlocked2.unblock.accesskey "D">
>  <!ENTITY mixedContentBlocked2.block.label "Enable protection">
> +<!ENTITY mixedContentBlocked2.block.accesskey "E">

We don't need to change the entity name here because accesskeys are based off of the related label, but it wouldn't hurt to send an email to the l10n mailing list announcing that mixedContentBlocked2.block.accesskey has changed.

@@ +739,5 @@
>  <!ENTITY trackingContentBlocked.moreinfo "Parts of the page that track your online activity have been blocked.">
>  <!ENTITY trackingContentBlocked.learnMore "Learn More">
>  <!ENTITY trackingContentBlocked.options "Options">
>  <!ENTITY trackingContentBlocked.unblock.label2 "Disable protection for this site">
> +<!ENTITY trackingContentBlocked.unblock.accesskey "D">

Use trackingContentBlocked.unblock.accesskey2 to be consistent with the 'label2' suffix above it.
Attachment #8488288 - Flags: review?(jaws) → review+
Assignee

Comment 3

5 years ago
trackingContentBlocked.unblock.accesskey >>> trackingContentBlocked.unblock.accesskey2 in urlbarbindings.xml and browser.dtd
Attachment #8488288 - Attachment is obsolete: true
Attachment #8489472 - Flags: review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
Comment on attachment 8489472 [details] [diff] [review]
Add access keys for tracking protection doorhanger

Review of attachment 8489472 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +739,5 @@
>  <!ENTITY trackingContentBlocked.moreinfo "Parts of the page that track your online activity have been blocked.">
>  <!ENTITY trackingContentBlocked.learnMore "Learn More">
>  <!ENTITY trackingContentBlocked.options "Options">
>  <!ENTITY trackingContentBlocked.unblock.label2 "Disable protection for this site">
> +<!ENTITY trackingContentBlocked.unblock.accesskey2 "D">

Actually, change trackingContentBlocked.unblock.accesskey2 to trackingContentBlocked.unblock2.accesskey and trackingContentBlocked.unblock.label2 to trackingContentBlocked.unblock2.label so they are consistent and both have a standard suffix.
Assignee

Comment 5

5 years ago
Attachment #8489472 - Attachment is obsolete: true
Attachment #8489602 - Flags: review?(jaws)
Attachment #8489602 - Flags: review?(jaws) → review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
Can we get a try run for this patch before asking for checkin-needed?
Keywords: checkin-needed
Assignee

Comment 7

5 years ago
(In reply to Wes Kocher (:KWierso) from comment #6)
> Can we get a try run for this patch before asking for checkin-needed?

Sorry Wes, you're right. Here is the try run for this patch:

https://tbpl.mozilla.org/?tree=Try&rev=e397e01d33fc

It looks good to me :-)
Assignee

Updated

5 years ago
Keywords: checkin-needed
Iteration: 35.1 → 35.2
https://hg.mozilla.org/mozilla-central/rev/c73b037b6dad
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
I've tested this using FF 35.0b5 and the only modifications I've noticed is that "D" and "E" chars are underlined on Ubuntu and Windows on the "Disable protection for this site" and "Enable protection" strings. Is this what the fix consists of or am I missing something?
Flags: needinfo?(alex.bardas)
Yes, and then pressing D and E while those elements are visible should activate them.  You might have to hold Ctrl too, I don't know.
Flags: needinfo?(alex.bardas)
Verified as fixed using:

FF 35.0b5
OS: Win 7 x64, Ubuntu 14.04 x64
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.