Closed Bug 1665066 Opened 4 years ago Closed 4 years ago

Remove payload.keywordOffer in favor of payload.providesSearchMode

Categories

(Firefox :: Address Bar, enhancement, P3)

Product:
Firefox
Component:
Address Bar
Type:
enhancement
Points:
2

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
firefox86 --- fixed

People

(Reporter: bugzilla, Assigned: amy, Mentored)

References

Details

Attachments

(1 file)

Bug 1665066 - Remove payload.keywordOffer in favor of payload.providesSearchMode. r?harry
48 bytes, text/x-phabricator-request
Details | Review

This is a follow-up item from this review comment. keywordOffer will no longer be a very descriptive title for this property once the update2 pref is removed. We should rename this property to payload.offersSearchModePreview.

Severity: -- → S3
Priority: -- → P3
Blocks: urlbar-update-2-follow-ups

Currently, keywordOffer is a property on UrlbarResult payloads. Its value is one of the values of UrlbarUtils.KEYWORD_OFFER. This bug entails updating most references to keywordOffers to be a boolean property called offersSearchModePreview. This will entail:

  1. Changing the property definition on UrlbarResultPayloads to be a boolean called offersSearchModePreview.
  2. Removing UrlbarUtils.KEYWORD_OFFER.
  3. Replacing references to keywordOffer: UrlbarUtils.KEYWORD_OFFER.SHOW/HIDE (example) and keywordOffer: UrlbarPrefs.get("update2") ? UrlbarUtils.KEYWORD_OFFER.SHOW : UrlbarUtils.KEYWORD_OFFER.HIDE (example) with simply offersSearchModePreview: true.
  4. Replacing the callers of UrlbarView._shouldLocalizeSearchResultTitle with a boolean check on offersSearchModePreview (you can also then remove _shouldLocalizeSearchResultTitle).
  5. Replacing boolean checks on keywordOffer to checks on offersSearchModePreview (example).
  6. Not replacing the "keywordoffer" string when it refers to a search mode entry point (example)
Mentor: htwyford

On Marco's suggestion, let's use the less-verbose providesSearchMode instead of offersSearchModePreview.

Summary: Remove payload.keywordOffer in favor of payload.offersSearchModePreview → Remove payload.keywordOffer in favor of payload.providesSearchMode
Assignee: nobody → achurchwell
Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c0cd17e0fc86
Remove payload.keywordOffer in favor of payload.providesSearchMode. r=harry

It seems quite unlikely that this patch caused those failures. I'll investigate tomorrow with Amy.

Taking a slightly wider view of autoland, it looks like the webdriver failures stopped shortly before this backout. Specifically, it looks like the backout of bug 1640607 fixed them. Could we try relanding this patch and seeing if the failures reoccur?

Flags: needinfo?(achurchwell) → needinfo?(malexandru)
Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/569af2e58d88
Remove payload.keywordOffer in favor of payload.providesSearchMode. r=harry

https://hg.mozilla.org/mozilla-central/rev/569af2e58d88

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox86: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

(In reply to Harry Twyford [:harry] from comment #7)

Taking a slightly wider view of autoland, it looks like the webdriver failures stopped shortly before this backout. Specifically, it looks like the backout of bug 1640607 fixed them. Could we try relanding this patch and seeing if the failures reoccur?

The webdriver tests that originally ran on the backout of Bug 1640607 did not contain the failing test suite, I have added those for clarification:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=420898d8b7b62c932b90a9a4e42a2a56df50829b&searchStr=wd&tochange=6f4a6f23eaf9ca2ddf6dd2b1153f21b77021e308&selectedTaskRun=WbvGbxfSTBi6xDswVkot_w.0

Flags: needinfo?(malexandru)
You need to log in before you can comment on or make changes to this bug.