1501234 - gAccessibilityServiceIndicator should use openTrustedLinkIn

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P3)

Description

[Dão Gottwald [:dao]]

+++ 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

[Md Shahazad Uddin]

I am new here . I want to work on this bug . Looking for mentorship to get started. Thank you.

Comment 2

[Dão Gottwald [:dao]]

(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

[Md Shahazad Uddin]

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.

Comment 4

[hereissophie]

(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

[Md Shahazad Uddin]

(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.

Comment 6

[hereissophie]

(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!

Comment 7

[Dão Gottwald [:dao]]

hereissophie, are you still interested in working on this?

Comment 8

[hereissophie]

(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

Comment 9

[hereissophie]

Hey Dão,

How could I verify that my fix is working? Thanks!

Comment 10

[hereissophie]

Attachment: bug1501234.patch

Please let me know if the patch is correct

Comment 11

[Dão Gottwald [:dao]]

(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

Comment 12

[hereissophie]

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?

Comment 13

[Dão Gottwald [:dao]]

(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

Comment 14

[Dão Gottwald [:dao]]

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

I tested this and it works. Thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d18e3cc2eaeb
Use openTrustedLinkIn in gAccessibilityServiceIndicator. r=dao

Comment 16

[hereissophie]

Thank you!

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d18e3cc2eaeb

Comment 18

[Emil Ghitta, QA [:emilghitta]]

Reproducible steps are provided in comment 11