963945 - In priv mode can open multiple addon tabs with shift+a

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[noitidart]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131205075310

Steps to reproduce:

press ctrl+shift+a in private mode multiple times


Actual results:

multiple addon tabs are opened


Expected results:

only one should have opened. if there was an addon manager tab in non-priv mode it should have focused that

Comment 1

[sys.sgx]

It's confirmed on FF29 Win 7.

Comment 2

[Josh Matthews [:jdm]]

I believe this could be as easy as adding about:addons to the list of whitelisted urls in switchToTabHavingURI (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7485). If that doesn't work, we may need to investigate what's going wrong in BrowserOpenAddonsMgr (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6541), but it should be easy enough to reproduce this problem and test out my proposed solution.

Comment 3

[Mike Lopez]

I would like to take this on. Thanks.

Comment 4

[Mike Lopez]

Attachment: First attempt at fix

First attempt at fix

Comment 5

[Josh Matthews [:jdm]]

Comment on attachment 8536969 [details] [diff] [review]
First attempt at fix

This patch is great, and it even includes a test! Thank you Michael! You can add a "r=jdm" suffix to the commit message so this is ready to merge :)

Comment 6

[Josh Matthews [:jdm]]

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

Comment 7

[Josh Matthews [:jdm]]

I've pushed this to our automated testing infrastructure; you should consider applying for access too: https://www.mozilla.org/hacking/committer/

Comment 8

[Mike Lopez]

Attachment: add reviewers suffix to message

Adding reviewers to message as requested.

Comment 9

[Josh Matthews [:jdm]]

The results from the push to try are all green, so this is ready to be checked in!

Comment 10

[Josh Matthews [:jdm]]

Comment on attachment 8536969 [details] [diff] [review]
First attempt at fix

For future reference, when attaching a new version it's a good practice to mark the old one as obsolete.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/7ad4cbf21299

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7ad4cbf21299

Comment 13

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Setting as qe-verify- since this has a test (browser/base/content/test/general/browser_bug963945.js).