gAccessibilityServiceIndicator should use openTrustedLinkIn

RESOLVED FIXED in Firefox 65

Status

()

enhancement
P3
normal
RESOLVED FIXED
6 months ago
4 months ago

People

(Reporter: dao, Assigned: hereissophie, Mentored)

Tracking

({good-first-bug})

Trunk
Firefox 65
Points:
---

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
+++ This bug was initially created as a clone of Bug #1500136 +++

gBrowser.selectedTab = gBrowser.addTab(...) is an antipattern as it doesn't set the owner tab for going back to the previous tab when closing the added one. openTrustedLinkIn takes care of this automatically.

This needs to be fixed here: https://searchfox.org/mozilla-central/rev/fcfb479e6ff63aea017d063faa17877ff750b4e5/browser/base/content/browser.js#7558

See bug 1500136 for another example.

Comment 1

6 months ago
I am new here . I want to work on this bug . Looking for mentorship to get started. Thank you.
(Reporter)

Comment 2

6 months ago
(In reply to shanudjn from comment #1)
> I am new here . I want to work on this bug . Looking for mentorship to get
> started. Thank you.

Hi. Have you read https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Source_Code or a similar guide?

Comment 3

6 months ago
I am starting with the building of firefox from the above mentioned link. Before moving forward with the bug fixing I will let you know . Need some guidance for that as I am new here.
(Assignee)

Comment 4

5 months ago
(In reply to shanudjn from comment #3)
> I am starting with the building of firefox from the above mentioned link.
> Before moving forward with the bug fixing I will let you know . Need some
> guidance for that as I am new here.

shanudjn are you still working on this bug? If not I'd like to take a crack at it. Thank you!

Comment 5

5 months ago
(In reply to hereissophie from comment #4)
> (In reply to shanudjn from comment #3)
> > I am starting with the building of firefox from the above mentioned link.
> > Before moving forward with the bug fixing I will let you know . Need some
> > guidance for that as I am new here.
> 
> shanudjn are you still working on this bug? If not I'd like to take a crack
> at it. Thank you!

Yes you can carry on with it! Since I am new here I am working on another bug (a much easier one,i think) to get started. Feel free to work on it.
(Assignee)

Comment 6

5 months ago
(In reply to shanudjn from comment #5)
> (In reply to hereissophie from comment #4)
> > (In reply to shanudjn from comment #3)
> > > I am starting with the building of firefox from the above mentioned link.
> > > Before moving forward with the bug fixing I will let you know . Need some
> > > guidance for that as I am new here.
> > 
> > shanudjn are you still working on this bug? If not I'd like to take a crack
> > at it. Thank you!
> 
> Yes you can carry on with it! Since I am new here I am working on another
> bug (a much easier one,i think) to get started. Feel free to work on it.

Great! Thank you so much! And good luck with your other bug!
(Reporter)

Comment 7

5 months ago
hereissophie, are you still interested in working on this?
Flags: needinfo?(hereissophie)
(Assignee)

Comment 8

5 months ago
(In reply to Dão Gottwald [::dao] from comment #7)
> hereissophie, are you still interested in working on this?

Hi Dão, yes I'm still interested, I'm aiming to submit a patch by the end of the next week
Flags: needinfo?(hereissophie)
(Assignee)

Comment 9

5 months ago
Hey Dão,

How could I verify that my fix is working? Thanks!
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 10

5 months ago
Please let me know if the patch is correct
(Reporter)

Comment 11

5 months ago
(In reply to hereissophie from comment #9)
> Hey Dão,
> 
> How could I verify that my fix is working? Thanks!

1. open about:config and set accessibility.indicator.enabled to true
2. open the browser console and run this:
     gAccessibilityServiceIndicator._update(true)
3. click the blue icon that should show up in the top right (on Windows & Linux) or left (on Mac) corner of the browser window
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 12

5 months ago
Hey Dão,

After running the test I got "ReferenceError: gAccessibilityServiceIndicator is not defined"

However I followed the exact same pattern as the example bug and changed "gBrowser.selectedTab = gBrowser.addTab(a11yServicesSupportURL)" to "openTrustedLinkIn(a11yServicesSupportURL, "tab");". I couldn't figure out what's wrong, do you have any ideas?
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 13

5 months ago
(In reply to hereissophie from comment #12)
> Hey Dão,
> 
> After running the test I got "ReferenceError: gAccessibilityServiceIndicator
> is not defined"
> 
> However I followed the exact same pattern as the example bug and changed
> "gBrowser.selectedTab = gBrowser.addTab(a11yServicesSupportURL)" to
> "openTrustedLinkIn(a11yServicesSupportURL, "tab");". I couldn't figure out
> what's wrong, do you have any ideas?

Are you sure you ran this in the browser console? See https://developer.mozilla.org/en-US/docs/Tools/Browser_Console
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 14

5 months ago
Comment on attachment 9029491 [details] [diff] [review]
bug1501234.patch

I tested this and it works. Thanks!
Attachment #9029491 - Flags: review+
(Reporter)

Updated

5 months ago
Assignee: nobody → hereissophie

Comment 15

5 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d18e3cc2eaeb
Use openTrustedLinkIn in gAccessibilityServiceIndicator. r=dao
(Assignee)

Comment 16

5 months ago
Thank you!

Comment 17

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d18e3cc2eaeb
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Reproducible steps are provided in comment 11
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.