webRequest.RequestFilter doesn't support tabId or windowId values

RESOLVED FIXED in Firefox 53

Status

P2
normal
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: 21Naown, Assigned: mixedpuppy)

Tracking

({dev-doc-complete})

unspecified
mozilla53
dev-doc-complete

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: triaged)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
For example:

chrome.webRequest.onResponseStarted.addListener(getURL,	{urls: ["TO_MATCH"], tabId: 22}, ["responseHeaders"]);

work out of tabId 22.

Updated

2 years ago
Assignee: nobody → mixedpuppy
Priority: -- → P2
Whiteboard: triaged
(Assignee)

Comment 1

2 years ago
expanding this to include both missing items.
Summary: chrome.webRequest.*.addListener doesn't support tabId value → webRequest.RequestFilter doesn't support tabId or windowId values
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1313523
(Assignee)

Comment 3

2 years ago
Created attachment 8807390 [details] [diff] [review]
WIP webrequestFilter

Wondering about the approach here.

One approach would be to just do this in WebRequest.jsm, but it doesn't seem right to pull in ext-* stuff there.  So this is a later filtering than the filtering done in WebRequest.jsm.
Attachment #8807390 - Flags: feedback?(kmaglione+bmo)
(Reporter)

Comment 4

2 years ago
I forgot to add that in the doc (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest#Chrome_incompatibilities), the problem is already reported. May be you known it too?
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8807390 - Attachment is obsolete: true
Attachment #8807390 - Flags: feedback?(kmaglione+bmo)
(Assignee)

Comment 6

2 years ago
Building the tests on top of patches in bug 1314492
Depends on: 1314492

Comment 7

2 years ago
mozreview-review
Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

https://reviewboard.mozilla.org/r/90792/#review90670

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:22
(Diff revision 1)
> +
> +add_task(function* setup() {
> +  testWindow = window.open("about:blank", "_blank", "width=100,height=100");
> +  yield waitForLoad(testWindow);
> +
> +  // fetch the windowId and tabId we need to filter with WebRequest

Nit: Please capitalize comments and end with full stop.

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:31
(Diff revision 1)
> +        "tabs",
> +      ],
> +    },
> +    background() {
> +      browser.windows.getCurrent({populate: true}).then(window => {
> +        browser.test.log(`current window ${window.id} tabs: ${JSON.stringify(window.tabs.map((tab) => { return [tab.id, tab.url]; }))}`);

Nit: `.map(tab => [tab.id, tab.url])`

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:38
(Diff revision 1)
> +      });
> +    },
> +  });
> +  yield extension.startup();
> +  windowData = yield extension.awaitMessage("windowData");
> +  info(`window is is ${JSON.stringify(windowData)}`);

s/is is/is/

::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html:48
(Diff revision 1)
> +  yield SpecialPowers.pushPrefEnv({
> +    set: [["dom.serviceWorkers.testing.enabled", true]],
> +  });
> +
> +  let events = {
> +    "onBeforeRequest":     [{urls: ["<all_urls>"], windowId: windowData.windowId}, ["blocking", "requestBody"]],

Probably best to leave "requestBody" out, and none of these probably need to be blocking.
Attachment #8807749 - Flags: review?(kmaglione+bmo) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/15886f6c3dd1
fix webrequest filter for tabId and windowId, r=kmag
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 14

2 years ago
Kris, can you look at the test changes in the last push to review?
Flags: needinfo?(mixedpuppy) → needinfo?(kmaglione+bmo)

Comment 15

2 years ago
mozreview-review
Comment on attachment 8807749 [details]
Bug 1311576 fix webrequest filter for tabId and windowId,

https://reviewboard.mozilla.org/r/90792/#review93290

::: toolkit/components/extensions/test/mochitest/head_webrequest.js:163
(Diff revisions 4 - 5)
>        let expectedEvent = expected.events[0] == name;
>        if (expectedEvent) {
>          expected.events.shift();
>        } else {
> -        expectedEvent = expected.optional_events[0] == name;
> -        if (expectedEvent) {
> +        // e10s vs. non-e10s errors can end with either onCompleted or onErrorOccurred
> +        expectedEvent = expected.optional_events.indexOf(name) >= 0;

`expected.optional_events.includes(name)`
Looks OK to me
Flags: needinfo?(kmaglione+bmo)
Comment hidden (mozreview-request)

Comment 18

2 years ago
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b227cc95b6d9
fix webrequest filter for tabId and windowId, r=kmag
Keywords: dev-doc-needed

Comment 19

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b227cc95b6d9
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
I've updated the compat data for this change. I think that's all we need, so am marking this dev-doc complete.
Keywords: dev-doc-needed → dev-doc-complete

Updated

6 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.