Closed Bug 1520368 Opened 5 years ago Closed 5 years ago

Clarify the queryContext.autofill property and add an enableAutofill property

Categories

(Firefox :: Address Bar, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 66
Tracking Status
firefox66 --- fixed

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(1 file)

From bug 1520342 comment 2:

(In reply to Drew Willcoxon :adw from bug 1520342 comment #2)

One related thing is that inside UrlbarController, context.autofill is set
like this:

queryContext.autofill = UrlbarPrefs.get("autoFill");

i.e., it means whether autofill is enabled.

But in UrlbarProviderUnifiedComplete, it's set if the first result is an
autofill result.

So there's a disconnect in what this property means. The rst doc says it's
"whether the first match is an autofill match", and I think clearly that's
what it should mean, not whether autofill is enabled.

I'm not sure whether we really need a context.enableAutofill property, but we have isPrivate, maxResults, etc., so we should probably add that property and make it required, like how the others are required.

And then we'd still have the autofill property, which means whether the first result is an autofill result, like the doc says and the UnifiedComplete provider assumes.

This patch is based on the patch in bug 1520342.

I made the UnifiedComplete provider manually check `context.enableAutofill` before setting `context.autofill`.  If we end up with other providers setting `autofill`, they'd have to be careful to check `enableAutofill` too.  Maybe it would be better to have a `context.autofill` getter that always returns false when `enableAutofill` is false, or a setter that forces it to be false in that case?  Anyway, I opted for a simple approach in this patch.

The patch also rearranges properties so that they're listed in alphabetical order.  Not really necessary, but I think it's easier to pick out properties that way, and it's a logical order for adding more properties.
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f25f8bee25b
Clarify the queryContext.autofill property and add an enableAutofill property. r=mak
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: