Closed
Bug 1041443
Opened 7 years ago
Closed 6 years ago
UnifiedComplete: What's _addFrecencyMatch sorting
Categories
(Toolkit :: Places, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: mano, Unassigned)
References
Details
_frecnecyMatches is directly manipulated only by _addFrecencyMatch, which is only called for prioritized urls... which have a constant frecency (the _process methods aren't called for _frecencyMatches). However, for every new match _addFrecencyMatch attempts to resort the array, by frecency.
Updated•7 years ago
|
Blocks: UnifiedComplete
Comment 1•7 years ago
|
||
(In reply to Mano (needinfo? for any questions; not reading general bugmail) from comment #0) > _frecnecyMatches is directly manipulated only by _addFrecencyMatch, which is > only called for prioritized urls... which have a constant frecency (the > _process methods aren't called for _frecencyMatches). However, for every new > match _addFrecencyMatch attempts to resort the array, by frecency. I don't think we should rely on what PriorityURLProvider does, new providers may add variable frecency values and then the code would be doing the right thing, isn't it?
Reporter | ||
Comment 2•7 years ago
|
||
It's not PriorityURLProvider, it's the code inside UnifiedComplete that hardcodes it.
Reporter | ||
Comment 3•7 years ago
|
||
_matchPriorityUrl: function* () { if (!Prefs.autofillPriority) return; let priorityMatch = yield PriorityUrlProvider.getMatch(this._searchString); if (priorityMatch) { this._result.setDefaultIndex(0); this._addFrecencyMatch({ ... frecency: FRECENCY_PRIORITY_DEFAULT }); } },
Comment 4•7 years ago
|
||
frecency may still not be a const in future and then we'd have introduced a very hard to notice bug at that point, imo.
Reporter | ||
Comment 5•7 years ago
|
||
Your call. I just didn't get the feeling that _frecencyMatches is the door to all external stuff. That would be a remote match ;)
Comment 6•6 years ago
|
||
We removed _addFrecencyMatch, so no reason to fix this.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•