Closed
Bug 944450
Opened 12 years ago
Closed 11 years ago
Collect UITelemetry on number of tabs opened
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
RESOLVED
FIXED
Firefox 29
People
(Reporter: mconley, Assigned: mconley)
References
Details
(Whiteboard: [fixed-in-holly][good first verify])
Attachments
(2 files, 1 obsolete file)
1.44 KB,
patch
|
mconley
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
1.53 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
I think we need to clear this up a little before this can get written. Do we want to measure the number of tabs a tab is opened, or just count the number of tabs in all windows at the point of the ping?
Comment 1•12 years ago
|
||
We're interested in how many tabs people have, and whether that changes with Australis (because, for instance, background tabs are less intrusive), so the count of number of tabs (perhaps per-window?) at the point of the ping will give us a reasonable answer, I believe.
Assignee | ||
Comment 2•12 years ago
|
||
So would collecting an array of tab counts (1 count per window) be sufficient?
Flags: needinfo?(bwinton)
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #8346059 -
Flags: review?(gijskruitbosch+bugs)
Comment 5•12 years ago
|
||
Comment on attachment 8346059 [details] [diff] [review]
Patch v1 - for non-Australis
Review of attachment 8346059 [details] [diff] [review]:
-----------------------------------------------------------------
r+ whichever way you answer the below question, but I suspect visibleTabs.length is a more sensible number.
::: browser/modules/BrowserUITelemetry.jsm
@@ +517,5 @@
> + let openTabs = [];
> + while (winEnumerator.hasMoreElements()) {
> + let someWin = winEnumerator.getNext();
> + if (someWin.gBrowser) {
> + openTabs.push(someWin.gBrowser.tabs.length);
Interesting. Do we want to count hidden panorama groups that even the user might have forgotten about? Maybe we want visibleTabs here? Should we collect both?
Attachment #8346059 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Blake, do you have a position on that? I tend to agree with Gijs that visible tabs is what we want here.
Flags: needinfo?(bwinton)
Comment 7•12 years ago
|
||
Yeah, visible tabs sounds like what we're interested in.
(having said that, hidden tabs _would_ be another interesting thing to measure. :)
Flags: needinfo?(bwinton)
Assignee | ||
Comment 8•12 years ago
|
||
Alright, this records counts for both visible and hidden tabs.
Attachment #8346059 -
Attachment is obsolete: true
Attachment #8346087 -
Flags: review+
Assignee | ||
Comment 9•12 years ago
|
||
Landed on Holly as https://hg.mozilla.org/projects/holly/rev/03fdb3303796.
Thanks Gijs!
Whiteboard: [Australis:P1] → [Australis:P1][fixed-in-holly]
Assignee | ||
Comment 10•12 years ago
|
||
Comment on attachment 8346087 [details] [diff] [review]
Patch v1.1 - for non-Australis (r+'d by Gijs)
[Approval Request Comment]
Bug caused by (feature/regressing bug #):
None. This gives BrowserUITelemetry the capability of counting the number of visible and hidden tabs per window.
User impact if declined:
None.
Testing completed (on m-c, etc.):
Just manual testing at this point.
Risk to taking this patch (and alternatives if risky):
None.
String or IDL/UUID changes made by this patch:
None.
Attachment #8346087 -
Flags: approval-mozilla-beta?
Attachment #8346087 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 11•12 years ago
|
||
Removing Australis:P1 whiteboard tag because these already block an Australis:P1 bug, and being redundant isn't helpful.
Whiteboard: [Australis:P1][fixed-in-holly] → [fixed-in-holly]
Updated•12 years ago
|
Attachment #8346087 -
Flags: approval-mozilla-beta?
Attachment #8346087 -
Flags: approval-mozilla-beta+
Attachment #8346087 -
Flags: approval-mozilla-aurora?
Attachment #8346087 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 12•12 years ago
|
||
Landed in mozilla-beta as https://hg.mozilla.org/releases/mozilla-beta/rev/73aedac83ac7
status-firefox27:
--- → fixed
Assignee | ||
Comment 13•12 years ago
|
||
Landed in mozilla-aurora as https://hg.mozilla.org/releases/mozilla-aurora/rev/27909461e31a
status-firefox28:
--- → fixed
Assignee | ||
Comment 14•12 years ago
|
||
At least on OS X, with bug 952180 landed, the tabs will always return 1 active tab, and no hidden tabs, making this measurement useless.
So, do we:
1) Not care enough about this probe to change it?
2) Update the probe to count the tabs at some time between browser startup and before browser shutdown? If so, what is that time?
Flags: needinfo?(bwinton)
Comment 15•11 years ago
|
||
(In reply to Mike Conley (:mconley) - vacationing from Jan 4 - 12 from comment #14)
> At least on OS X, with bug 952180 landed, the tabs will always return 1
> active tab, and no hidden tabs, making this measurement useless.
Why? If you use tab groups, there should be hidden tabs, too... I just ran this:
let winEnumerator = Services.wm.getEnumerator("navigator:browser");
let visibleTabs = [];
let hiddenTabs = [];
while (winEnumerator.hasMoreElements()) {
let someWin = winEnumerator.getNext();
if (someWin.gBrowser) {
let visibleTabsNum = someWin.gBrowser.visibleTabs.length;
visibleTabs.push(visibleTabsNum);
hiddenTabs.push(someWin.gBrowser.tabs.length - visibleTabsNum);
}
} console.log(visibleTabs.join(','), hiddenTabs.join(','))
Which produces:
"47" "6"
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #15)
> (In reply to Mike Conley (:mconley) - vacationing from Jan 4 - 12 from
> comment #14)
> > At least on OS X, with bug 952180 landed, the tabs will always return 1
> > active tab, and no hidden tabs, making this measurement useless.
>
> Why? If you use tab groups, there should be hidden tabs, too...
Because bug 952180 made it so that we looked for these tabs immediately after first paint, which was too soon - the session hadn't been restored yet. This guaranteed an active tab count of 1 and a hidden tab count of 0.
Bug 956138 landed, however, which fixes this by making it so that we gather measurements after the sessionstore-windows-restored event fires.
Comment 17•11 years ago
|
||
So this is fixed, and I don't need to provide info? ;)
Flags: needinfo?(bwinton)
Assignee | ||
Comment 18•11 years ago
|
||
This is a straight-port from the non-Australis patch.
Attachment #8359252 -
Flags: review?(gijskruitbosch+bugs)
Comment 19•11 years ago
|
||
Comment on attachment 8359252 [details] [diff] [review]
Patch v1 - for Australis
Review of attachment 8359252 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM!
Thanks for the tireless work on the telemtry bugs, Mike. :-)
Attachment #8359252 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 20•11 years ago
|
||
Thanks for the reviews! :D
Landed on fx-team as https://hg.mozilla.org/integration/fx-team/rev/849d9644387c
Whiteboard: [fixed-in-holly][good first verify] → [fixed-in-holly][good first verify][fixed-in-fx-team]
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-holly][good first verify][fixed-in-fx-team] → [fixed-in-holly][good first verify]
Target Milestone: --- → Firefox 29
You need to log in
before you can comment on or make changes to this bug.
Description
•