tabs.onUpdated does not fire for title changes

RESOLVED FIXED in Firefox 53

Status

P2
normal
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: 21Naown, Assigned: zombie)

Tracking

unspecified
mozilla53
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: triaged)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Related to: https://developer.chrome.com/extensions/tabs#event-onUpdated
Firefox has not "title" for example.

Moreover, if you are not restricted by Chrome's specs, what about add a filter like https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onBeforeRequest#Parameters ?

Updated

2 years ago
Flags: needinfo?(kmaglione+bmo)
Status: UNCONFIRMED → NEW
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Ever confirmed: true
Flags: needinfo?(kmaglione+bmo)
Summary: tabs.onUpdated updatedInfo should contain more triggers and filter (may be) → tabs.onUpdated does not fire for title changes

Updated

2 years ago
Priority: -- → P2
Whiteboard: triaged
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Assignee: nobody → tomica
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 years ago
There are two approaches to this bug:
 - simpler one, observing the changes of the tab's label attribute, or
 - alternatively, observe "DOMTitleChanged" messages that wouldn't include things like "Connecting..." and "New Tab" which come from the browser UI.

I opted for the first one, because it makes more sense to me semantically.  This is about _tab_ title changes, not only document's title changes.  Also, we already return tab.label as tab's title if you query a tab at just the right time [1].

1) http://searchfox.org/mozilla-central/source/browser/components/extensions/ext-utils.js#686
(Assignee)

Updated

2 years ago
Attachment #8824166 - Flags: review?(kmaglione+bmo)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8824166 [details]
Bug 1320412 - Emit  tabs.onUpdated event for title changes

https://reviewboard.mozilla.org/r/102702/#review103050

::: browser/components/extensions/ext-tabs.js:436
(Diff revision 1)
>            } else if (event.type == "TabUnpinned") {
>              needed.push("pinned");
>            }
>  
>            if (needed.length && !extension.hasPermission("tabs")) {
>              needed = needed.filter(attr => attr != "url" && attr != "favIconUrl");

We also need to filter out "title" here. Please add a test that it's filtered out when tabs permissions are missing.

::: browser/components/extensions/test/browser/browser_ext_tabs_onUpdated.js:185
(Diff revision 1)
> +    const url = "http://mochi.test:8888/browser/browser/components/extensions/test/browser/context_tabs_onUpdated_page.html";
> +    const tab = await browser.tabs.create({url});
> +
> +    browser.tabs.onUpdated.addListener(function onUpdated(tabId, changeInfo) {
> +      browser.test.assertEq(tabId, tab.id, "Check tab id");
> +      browser.test.log("onUpdate: " + JSON.stringify(changeInfo));

Nit: onUpdated, and please use a template string.

::: browser/components/extensions/test/browser/browser_ext_tabs_onUpdated.js:187
(Diff revision 1)
> +
> +    browser.tabs.onUpdated.addListener(function onUpdated(tabId, changeInfo) {
> +      browser.test.assertEq(tabId, tab.id, "Check tab id");
> +      browser.test.log("onUpdate: " + JSON.stringify(changeInfo));
> +      if ("title" in changeInfo) {
> +        if (changeInfo.title === url) {

if ("title" in changeInfo && changeInfo.title !== ...)
Attachment #8824166 - Flags: review?(kmaglione+bmo) → review+

Updated

2 years ago
webextensions: --- → +
Comment hidden (mozreview-request)
(Assignee)

Comment 5

2 years ago
mozreview-review-reply
Comment on attachment 8824166 [details]
Bug 1320412 - Emit  tabs.onUpdated event for title changes

https://reviewboard.mozilla.org/r/102702/#review103050

> We also need to filter out "title" here. Please add a test that it's filtered out when tabs permissions are missing.

That was sloppy of me, sorry.

> if ("title" in changeInfo && changeInfo.title !== ...)

Turns out we also get "Connectiong..." on some platforms...
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 7

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ec8836909060
Emit  tabs.onUpdated event for title changes r=kmag
Keywords: checkin-needed

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ec8836909060
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Reporter)

Comment 9

2 years ago
> Moreover, if you are not restricted by Chrome's specs, what about add a filter like
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onBeforeRequest#Parameters
> ?

I assume you cannot check for this? Or should I create a specific report?
(Assignee)

Comment 10

2 years ago
Yeah, this bug got renamed to just fix the existing bug.  You should file a separate one to request the enhancements, so that a design decision can be made.

Updated

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