Re-use existing rows when receiving results
Categories
(Firefox :: Address Bar, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox68 | --- | fixed |
People
(Reporter: dao, Assigned: dao)
References
Details
(Keywords: perf)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Currently rows are always created from scratch when receiving results. We should try to re-use existing rows.
Assignee | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
I'd like to add one detail here, that may not be obvious from how we outlined this bug. Tests must be able to tell if a row is "current" (refers to the current query) or not.
In the old autocomplete, we reused richlistitems, and we didn't hide them until new results arrive (or when the query was done, or on a timer), but tests needed to differentiate between current or old rows. To do that each row had an an "ac-text" attribute, that we used to compare with the current search string, if they are the same the result refers to the running query.
We may want to somehow connect rows to specific queryContexts.
Comment 2•6 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #1)
I'd like to add one detail here, that may not be obvious from how we outlined this bug. Tests must be able to tell if a row is "current" (refers to the current query) or not.
In the old autocomplete, we reused richlistitems, and we didn't hide them until new results arrive (or when the query was done, or on a timer), but tests needed to differentiate between current or old rows. To do that each row had an an "ac-text" attribute, that we used to compare with the current search string, if they are the same the result refers to the running query.
We may want to somehow connect rows to specific queryContexts.
I can see the QuantumBar working the same way. Wouldn't it make sense - since we now have a QueryContext, not just a query text, to use an sequence ID that resets to 1 when it's >= MAX_INT?
This way it'll become a more logical operation, comparing numbers, here. WDYT?
Comment 3•6 years ago
|
||
Another detail that is not documented about the legacy urlbar: we distinguish results coming from the network from local results, because they have different characteristics.
For example, we put search suggestions before historical results, this means that every time the user types a new char, if remote results are slower to arrive, we'd end up adding historical results and then they would be pushed down by search suggestions, and this causes continuous flicker while typing.
What the old urlbar does is replacing same-type results, and once a search is done it removes the "stale" results that don't apply anymore. This prevents flicker of results, because before removing search suggestions the view waits for new ones to arrive (and often they may be the same already there).
This is implemented in UnifiedComplete in the legacy implementation, but in the Quantum Bar we can't reuse that, because the new API inserts new results one by one and lets the sorting to the muxer. Though, the view can likely do something like this using the result type/source.
This flicker prevention is probably more important than any resource saved by reusing elements.
Assignee | ||
Comment 4•6 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #3)
Another detail that is not documented about the legacy urlbar: we distinguish results coming from the network from local results, because they have different characteristics.
For example, we put search suggestions before historical results, this means that every time the user types a new char, if remote results are slower to arrive, we'd end up adding historical results and then they would be pushed down by search suggestions, and this causes continuous flicker while typing.
What the old urlbar does is replacing same-type results, and once a search is done it removes the "stale" results that don't apply anymore. This prevents flicker of results, because before removing search suggestions the view waits for new ones to arrive (and often they may be the same already there).This is implemented in UnifiedComplete in the legacy implementation, but in the Quantum Bar we can't reuse that, because the new API inserts new results one by one and lets the sorting to the muxer. Though, the view can likely do something like this using the result type/source.
This flicker prevention is probably more important than any resource saved by reusing elements.
My current plan is to make that a followup to this bug in bug 1535656.
Assignee | ||
Comment 5•6 years ago
|
||
Updated•6 years ago
|
Comment 7•6 years ago
|
||
Backed out for BC failures on browser_tabMatchesInAwesomebar.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/9f3caf4a14b09ca0705ca905d6b3aa321da59f56
Push link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&classifiedState=unclassified&revision=6385899fe7b9ccd58f9ea0091e9d158416f83605&selectedJob=236630818
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=236630818&repo=autoland&lineNumber=28940
Assignee | ||
Updated•6 years ago
|
Comment 9•6 years ago
|
||
bugherder |
Updated•6 years ago
|
Description
•