Open
Bug 1494103
Opened 6 years ago
Updated 2 years ago
Dynamically create the "sidebar" <browser> when it's needed
Categories
(Firefox :: Bookmarks & History, task, P3)
Firefox
Bookmarks & History
Tracking
()
NEW
People
(Reporter: bgrins, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Right now we put the sidebar browser in browser.xul and then are careful to not eagerly access it from JS to avoid attaching XBL.
- https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/browser/base/content/browser.xul#1285
- https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/browser/base/content/browser-sidebar.js#43
To make this more compatible with Custom Elements (which get constructed regardless of element visibility or js reference), we could change this around to dynamically make the browser element when needed. I'm hoping this won't be too difficult, since we're already lazily accessing it.
Reporter | ||
Comment 1•6 years ago
|
||
Oddly, this seems to cause perf regressions https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=0e59d66dd39316efafdaefab76a16160f9d92130&newProject=try&newRevision=4f27edf059510d900af9610d3af2724eabbc4c9d&framework=1&showOnlyImportant=1. My first guess is that having it in the markup is causing browser.xml to be loaded eagerly, or something. Even though the binding isn't attached until the sidebar is opened, which I assume talos tests don't do.
Reporter | ||
Comment 2•6 years ago
|
||
Actually, it looks like the straightforward CE migration isn't causing any talos regression. Given that plus Comment 1, I'm going to close this. We may ultimately want to lazify this anyway, but it shouldn't block migration.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Reporter | ||
Comment 3•5 years ago
|
||
Reporter | ||
Updated•5 years ago
|
Assignee: nobody → bgrinstead
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Reporter | ||
Updated•5 years ago
|
Status: REOPENED → ASSIGNED
Type: defect → task
Updated•5 years ago
|
Priority: -- → P3
Reporter | ||
Comment 4•5 years ago
|
||
I don't have time to work on this right now, so marking as unassigned. I still think this should help with performance since we won't be running the browser constructor at startup, but I'm seeing some test failures (at least some of which should be test fixes due to some tests expecting the node to be there before the sidebar is opened).
Assignee: bgrinstead → nobody
Status: ASSIGNED → NEW
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•