Clarify the queryContext.autofill property and add an enableAutofill property
Categories
(Firefox :: Address Bar, enhancement, P1)
Tracking
()
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.
Assignee | ||
Comment 1•5 years ago
|
||
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
Comment 3•5 years ago
|
||
bugherder |
Description
•