Closed Bug 1041443 Opened 10 years ago Closed 10 years ago

UnifiedComplete: What's _addFrecencyMatch sorting

Categories

(Toolkit :: Places, defect)

x86
macOS
defect
Not set
normal

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.
(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?
It's not PriorityURLProvider, it's the code inside UnifiedComplete that hardcodes it.
_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 }); } },
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.
Your call. I just didn't get the feeling that _frecencyMatches is the door to all external stuff. That would be a remote match ;)
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.