Closed Bug 1432263 Opened 2 years ago Closed 2 years ago

Make matchBuckets string parsing the same everywhere it's done

Categories

(Firefox :: Address Bar, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox59 --- fixed
firefox60 --- fixed

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(2 files)

Bug 1426216 added its own matchBuckets parsing in order to determine whether a checkbox in the search preferences UI should be checked, and it's slightly different from the parsing that UnifiedComplete does.  It's possible for the checkbox not to reflect reality because of this difference.  In practice that shouldn't happen, though.  The only way for it to happen is if the user edited browser.urlbar.matchBuckets and didn't stick to the exact format that the checkbox expects.  But, there's no reason for these two places in the code to use different ways of parsing these strings.
Comment on attachment 8944504 [details]
Bug 1432263 - Make matchBuckets string parsing the same everywhere it's done.

https://reviewboard.mozilla.org/r/214686/#review220468
Attachment #8944504 - Flags: review?(mak77) → review+
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d957a46ed50c
Make matchBuckets string parsing the same everywhere it's done. r=mak
https://hg.mozilla.org/mozilla-central/rev/d957a46ed50c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Attached patch Beta patchSplinter Review
Approval Request Comment
[Feature/Bug causing the regression]:
This isn't a regression, but it's a followup from bug 1426216

[User impact if declined]:
None for most users.  Only users who manually edited the browser.urlbar.matchBuckets pref, or perhaps had an add-on or study that did, would possibly be affected.  The reason I'm asking for uplift is because there's no reason for this patch not to ship in the same release as the feature it's related to (the aforementioned bug 1426216)

[Is this code covered by automated tests?]:
Yes

[Has the fix been verified in Nightly?]:
No

[Needs manual test from QE? If yes, steps to reproduce]:
No

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
No

[Why is the change risky/not risky?]:
Small patch that simply factors out some common code

[String changes made/needed]:
None
Attachment #8945153 - Flags: approval-mozilla-beta?
Comment on attachment 8945153 [details] [diff] [review]
Beta patch

Sounds like we should ship this fix in 59 to make sure UI behavior is consistent. Let's uplift for the beta 4 build.
Attachment #8945153 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.