Closed Bug 1225215 Opened 4 years ago Closed 3 years ago

BrowserAction setter methods change default value when tab ID is invalid

Categories

(WebExtensions :: Untriaged, defect, P2)

defect

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Iteration:
47.3 - Mar 7
Tracking Status
firefox51 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

Details

(Whiteboard: [browserAction]triaged)

Attachments

(1 file)

We change the default values when the tab passed to `setProperty` is null. This is only meant to happen for explicitly omitted tab IDs, but `TabManager.getTab` also returns null for invalid IDs.
Chrome sets lastError in these cases, so we're going to need to wait for bug 1190680. And we'll need to make sure lastError is cleared before and after entering any API functions, so we'll also need to wait for the binding layer in bug 1208257.
Depends on: 1190680, 1208257
Iteration: --- → 47.3 - Mar 7
Priority: -- → P2
Whiteboard: [browserAction] → [browserAction]triaged
Comment on attachment 8760039 [details]
Bug 1225215: [webext] Raise the expected errors when given invalid tab IDs.

https://reviewboard.mozilla.org/r/57766/#review54838

::: browser/components/extensions/ext-tabs.js:835
(Diff revision 1)
>              move([tabA, tabB], {index: 0})
>                -> tabA to 0, tabB to 0 if tabA and tabB are in different windows
>          */
>          let indexMap = new Map();
>  
> -        for (let tabId of tabIds) {
> +        let tabs = tabIds.map(tabId => TabManager.getTab(tabId, context));

Did you mean to get rid of the previous "ignore invalid tabs" behavior?

::: toolkit/components/extensions/ext-webNavigation.js:166
(Diff revision 1)
>          if (!tab) {
>            return Promise.reject({message: `No tab found with tabId: ${details.tabId}`});
>          }

i don't think this is reachable any more?

::: toolkit/components/extensions/ext-webNavigation.js:178
(Diff revision 1)
>          if (!tab) {
>            return Promise.reject({message: `No tab found with tabId: ${details.tabId}`});
>          }

same as above
Attachment #8760039 - Flags: review?(aswan) → review+
https://reviewboard.mozilla.org/r/57766/#review54838

> Did you mean to get rid of the previous "ignore invalid tabs" behavior?

Yeah. I never saw the point of that, and we didn't have a test for it, so I figured we may as well make the behavior consistent with the rest of the tab methods.

> i don't think this is reachable any more?

Oh, yeah.
https://hg.mozilla.org/integration/mozilla-inbound/rev/57c5749e5b19612638a3ab08027431e6346c538e
Bug 1225215: [webext] Raise the expected errors when given invalid tab IDs. r=aswan
https://hg.mozilla.org/mozilla-central/rev/57c5749e5b19
https://hg.mozilla.org/mozilla-central/rev/231c74115211
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.