Closed
Bug 1041443
Opened 10 years ago
Closed 10 years ago
UnifiedComplete: What's _addFrecencyMatch sorting
Categories
(Toolkit :: Places, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: asaf, 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•10 years ago
|
Blocks: UnifiedComplete
Comment 1•10 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•10 years ago
|
||
It's not PriorityURLProvider, it's the code inside UnifiedComplete that hardcodes it.
Reporter | ||
Comment 3•10 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•10 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•10 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•10 years ago
|
||
We removed _addFrecencyMatch, so no reason to fix this.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•