Closed Bug 1320564 Opened 3 years ago Closed 3 years ago

Fix new tab default value logic for cancelEditMode

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(1 file)

I'm not quite sure what I was thinking here (https://dxr.mozilla.org/mozilla-central/rev/2a0abcff5cfce087c12f3e4820b5e8b773cffaca/mobile/android/chrome/content/browser.js#3496) - I think the intention was that cancelEditMode should imply "selected", but the code there doesn't work for that purpose.

There's no harm done here, because our only caller does the correct thing anyway, but we should nevertheless fix this.
Comment on attachment 8814711 [details]
Bug 1320564 - Fix the default value logic for "selected" when creating a new tab.

https://reviewboard.mozilla.org/r/95818/#review96062

::: mobile/android/chrome/content/browser.js:3497
(Diff revision 1)
>          uri: truncate(uri, MAX_URI_LENGTH),
>          parentId: ("parentId" in aParams) ? aParams.parentId : -1,
>          tabIndex: ("tabIndex" in aParams) ? aParams.tabIndex : -1,
>          external: ("external" in aParams) ? aParams.external : false,
> -        selected: ("selected" in aParams || aParams.cancelEditMode === true) ? aParams.selected : true,
> +        selected: ("selected" in aParams || aParams.cancelEditMode === true)
> +                  ? aParams.selected !== false || aParams.cancelEditMode === true : true,

Any reason why it's selected !== false and not selected === true?
Attachment #8814711 - Flags: review?(s.kaspari) → review+
Comment on attachment 8814711 [details]
Bug 1320564 - Fix the default value logic for "selected" when creating a new tab.

https://reviewboard.mozilla.org/r/95818/#review96062

> Any reason why it's selected !== false and not selected === true?

The idea was that if we aParams.selected ever ends up as explicitly `undefined` (I think I managed to do something similar some time ago, see bug 1274390), we still pass on the default value.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6b4ddaf6c276
Fix the default value logic for "selected" when creating a new tab. r=sebastian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6b4ddaf6c276
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.