Closed
Bug 1447723
Opened 5 years ago
Closed 5 years ago
page action commands dont work when using pattern matching
Categories
(WebExtensions :: Untriaged, defect, P1)
Tracking
(firefox-esr52 unaffected, firefox59 wontfix, firefox60+ fixed, firefox61 fixed)
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox59 | --- | wontfix |
firefox60 | + | fixed |
firefox61 | --- | fixed |
People
(Reporter: mixedpuppy, Assigned: mixedpuppy)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
rpl
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
Page action command fails in an extension that uses page action pattern matching and a key command with _execute_page_action. manifest: "page_action": { "default_popup": "page.html", "show_matches": ["<all_urls>"], "browser_style": true }, "commands": { "_execute_page_action": { "suggested_key": { "default": "Alt+Shift+3" } } } This is a regression starting in fx59 from bug 1437178.
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → mixedpuppy
Priority: -- → P1
Comment 1•5 years ago
|
||
Yeah, sorry, forgot this. https://searchfox.org/mozilla-central/rev/b29daa46443b30612415c35be0a3c9c13b9dc5f6/browser/components/extensions/ext-pageAction.js#232 pageAction.getProperty(window.gBrowser.selectedTab, "show") should now be pageAction.isShown(window.gBrowser.selectedTab)
Comment hidden (mozreview-request) |
Comment 3•5 years ago
|
||
mozreview-review |
Comment on attachment 8961055 [details] Bug 1447723 - fix triggering pageAction when using pattern matching, https://reviewboard.mozilla.org/r/229808/#review235590 ::: browser/components/extensions/test/browser/browser_ext_commands_execute_page_action.js:137 (Diff revision 1) > await extension.awaitFinish("page-action-with-popup"); > await extension.unload(); > }); > + > +add_task(async function test_execute_page_action_with_matching() { > + let scriptPage = url => `<html><head><meta charset="utf-8"><script src="${url}"></script></head><body>Test Popup</body></html>`; Nit, we could just share one of these scriptPage helpers with the previous test by defining it as a let (or const) global.
Attachment #8961055 -
Flags: review?(lgreco) → review+
Comment 4•5 years ago
|
||
mozreview-review |
Comment on attachment 8961055 [details] Bug 1447723 - fix triggering pageAction when using pattern matching, https://reviewboard.mozilla.org/r/229808/#review235596 ::: browser/components/extensions/ext-pageAction.js:237 (Diff revision 1) > */ > triggerAction(window) { > + if (this.isShown(window.gBrowser.selectedTab)) { > - let pageAction = pageActionMap.get(this.extension); > + let pageAction = pageActionMap.get(this.extension); > - if (pageAction.getProperty(window.gBrowser.selectedTab, "show")) { > pageAction.handleClick(window); `this` and `pageAction` seem to be the same object. If they are always the same, why bother defining `pageAction`? Otherwise, shouldn't it also be `pageAction.isShown`?
Comment 5•5 years ago
|
||
mozreview-review-reply |
Comment on attachment 8961055 [details] Bug 1447723 - fix triggering pageAction when using pattern matching, https://reviewboard.mozilla.org/r/229808/#review235596 > `this` and `pageAction` seem to be the same object. If they are always the same, why bother defining `pageAction`? Otherwise, shouldn't it also be `pageAction.isShown`? I think that you are right, and looking at the hg history triggerAction was actually like this from its first version (introduced by Bug 1246035), but even then it seems that retrieving the pageAction instance from the map wasn't really necessary.
Comment hidden (mozreview-request) |
Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/426e9698be8d fix triggering pageAction when using pattern matching, r=rpl
Comment 9•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/426e9698be8d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Comment 10•5 years ago
|
||
Comment on attachment 8961055 [details] Bug 1447723 - fix triggering pageAction when using pattern matching, Approval Request Comment [Feature/Bug causing the regression]: 1437178 [User impact if declined]: key commands are unable to open page action [Is this code covered by automated tests?]: yes, new test for key command to open page action but this change is also covered by any page action test that opens a page action. [Has the fix been verified in Nightly?]: by tests only [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: low [Why is the change risky/not risky?]: simple change [String changes made/needed]: none
Attachment #8961055 -
Flags: approval-mozilla-beta?
Comment 11•5 years ago
|
||
Comment on attachment 8961055 [details] Bug 1447723 - fix triggering pageAction when using pattern matching, webextensions regression fix, beta60+
Attachment #8961055 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•5 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/7f9921c6775a
Flags: in-testsuite+
Updated•5 years ago
|
status-firefox-esr52:
--- → unaffected
Updated•4 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•