Closed
Bug 1491784
Opened 6 years ago
Closed 6 years ago
_handleTabSelect accesses selectedItem too often
Categories
(Firefox :: Tabbed Browser, enhancement, P3)
Firefox
Tabbed Browser
Tracking
()
RESOLVED
FIXED
Firefox 64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: dao, Assigned: maggus.staab)
References
Details
(Keywords: perf)
Attachments
(1 file, 1 obsolete file)
+++ This bug was initially created as a clone of Bug #1491286 +++
The selectedItem getter is kind of expensive, we should only access it once in _handleTabSelect:
https://searchfox.org/mozilla-central/rev/99cbc0aec3e1c0b65ff9052523fb5c181b248f57/browser/base/content/tabbrowser.xml#400,402
See the patch in bug 1491286 for how to fix this.
Assignee | ||
Comment 1•6 years ago
|
||
I will send a patch for this.
Assignee | ||
Comment 2•6 years ago
|
||
I guess I did not get everything right, but the diff is on phabricator now:
https://phabricator.services.mozilla.com/D6287
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 3•6 years ago
|
||
Assignee | ||
Comment 4•6 years ago
|
||
Reporter | ||
Comment 5•6 years ago
|
||
Comment on attachment 9010365 [details]
Bug 1491784 - Access selectedItem only once in _handleTabSelect. r=dao
Dão Gottwald [::dao] has approved the revision.
Attachment #9010365 -
Flags: review+
Reporter | ||
Comment 6•6 years ago
|
||
Attachment 9010365 [details] looks good. You can abandon attachment 9010368 [details].
Gonna land this in a bit.
Thanks!
Assignee: nobody → maggus.staab
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 7•6 years ago
|
||
sorry for the double post. I am new to hg and mixed some commands up, because I am used to git and it seems the commands are not equivalent for some parts.
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f51b39c98c0
Access selectedItem only once in _handleTabSelect. r=dao
Assignee | ||
Updated•6 years ago
|
Attachment #9010368 -
Attachment is obsolete: true
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #6)
> You can abandon attachment 9010368 [details]
just marked the attachment as obsolete. didn't found a button or similar to close the corresponding diff in phabricator
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(dao+bmo)
Reporter | ||
Comment 10•6 years ago
|
||
Yeah, no clue how to do that in phabricator. I guess you can just leave the diff open.
Flags: needinfo?(dao+bmo)
Comment 11•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in
before you can comment on or make changes to this bug.
Description
•