UnifiedComplete: What's _addFrecencyMatch sorting

RESOLVED INVALID

Status

()

Product:
Toolkit
Component:
Places
Type:
defect
Status:
RESOLVED INVALID
Opened:
5 years ago
Updated:
5 years ago

People

(Reporter: mano, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

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: 5 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.