[commands] Handle _execute_browser_action correctly when a pageAction popup is displayed.

NEW
Unassigned

Status

()

Toolkit
WebExtensions: Frontend
P3
normal
9 months ago
2 days ago

People

(Reporter: mattw, Unassigned, Mentored)

Tracking

(Blocks: 2 bugs, {good-first-bug})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [commands] triaged)

(Reporter)

Description

9 months ago
As pointed out by Gijs in https://bugzilla.mozilla.org/show_bug.cgi?id=1246034#c9, it may be possible for _execute_browser_action to close the PageAction popup if it is showing.  We should make sure that this won't happen and add in the necessary tests to confirm it.
I think the correct behavior is actually to close the pageAction popup and then execute the browserAction.
Component: WebExtensions → WebExtensions: Frontend

Updated

9 months ago
Mentor: mwein@mozilla.com
Priority: -- → P3
Whiteboard: [commands] → [commands][good first bug] triaged
(Reporter)

Updated

9 months ago
Keywords: good-first-bug
Priority: P3 → --
Summary: [commands] Make sure _execute_browser_action only closes the browser action popup → [commands] Handle _execute_browser_action correctly when a pageAction popup is displayed.
Whiteboard: [commands][good first bug] triaged → [commands]
(Reporter)

Updated

9 months ago
Priority: -- → P3

Updated

9 months ago
Whiteboard: [commands] → [commands] triaged
Blocks: 1237377

Comment 2

6 months ago
Can i work on this as my first bug?
Thanks
Yes!  Again, ask if you get stuck.
Assignee: nobody → stevenellul
Status: NEW → ASSIGNED

Comment 4

6 months ago
Not really sure how to start here.

To me it looks like the first step is to try to get a browser action to close the pageAction popup before trying to account for that.

How do I recreate this possible issue?

Thanks
Flags: needinfo?(mwein)
(Reporter)

Comment 5

6 months ago
(In reply to Steven Ellul from comment #4)
> Not really sure how to start here.
> 
> To me it looks like the first step is to try to get a browser action to
> close the pageAction popup before trying to account for that.

I think this sounds like the right way to start.

The first step would be to add a test to http://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js which opens a page action popup and then uses the shortcut to execute the browser action. The test should check to see if the browser action popup is shown and since this isn't fixed yet the test should fail.

The second step would be to fix the issue by closing the page action popup before executing the browser action. I think the code to close the page action popup would go before this line here: https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/ext-commands.js#133.

The last step would be to confirm that the test passes now. If so, you're all done :)

Here's some more information on how to write the test:
 - An example test to follow: http://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js#106

 - How to handle options passed into the test: http://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js#21

 - To open the page action popup you'll first need to show the page action - http://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_commands_execute_page_action.js#101, and then you can open it using https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_pageAction_popup.js#146.

Writing the test for this might be a little tricky, so please ping me if you would like any more help (my irc nick is `k9`).
Flags: needinfo?(mwein)
(Reporter)

Comment 6

6 months ago
I'm usually around in #webextensions, and I'm sure others there would be happy to help out as well.

Comment 7

5 months ago
Unassigned myself.

Sorry to have prevented others working on this, but I was a bit silly asking to be assigned to this bug because it is outside of my skill set.
Status: ASSIGNED → NEW

Updated

5 months ago
Assignee: stevenellul → nobody
You need to log in before you can comment on or make changes to this bug.