1987298 - "Switch to tab" button is not shown when it should be

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:

1. Create a new profile.
2. Copy sessionstore.jsonlz4 to the new profile.
3. Start Firefox using the profile.
4. Choose Hamburger menu > History > Restore previous session.
5. Paste a URL that is already open in a restored tab. Make sure that "Switch to tab" button is shown in the dropdown.
6. Select all and drag the URL into the tab strip. A duplicate tab will open.
7. Close the tab.
8. Again paste the URL that was pasted in step 5.

Actual result:
"Switch to tab" is not shown.

Expected result:
"Switch to tab" should be shown.

Regression range:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=7fafc41c9dd59ac0fe9c4a3377097a4fb926e290&tochange=0543dcc503dcc2dd5a684546e7b8b51b3d097297

Edit: I could reproduce the bug with the clean profile.

Comment 1

[Masatoshi Kimura [:emk]]

good: d3beeb6bbbec0c8395b41d1142faa63492322a1d
bad: 0543dcc503dcc2dd5a684546e7b8b51b3d097297

Comment 2

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1908451

:jswinarton, since you are the author of the regressor, bug 1908451, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Masatoshi Kimura [:emk]]

With broken builds, moz_openpages_temp.open_count did not count up after the step 6. But the step 7 decrements open_count and therefore the record is removed by moz_openpages_temp_afterupdate_trigger.

Comment 4

[Masatoshi Kimura [:emk]]

SQLite allows NULL in primary key [1]. In this case, primary key is not unique (!)[2]. You can insert multiple records with the same primary key. So moz_openpages_temp does not work as expected.
IMO you should not use NULL for moz_openpages_temp.groupId to avoid this SQLite "feature". For example, use empty string to represent null group id on moz_openpages_temp and convert between empty string and null when reading or writing to the table.

[1] https://stackoverflow.com/questions/64753105/why-can-i-add-null-value-to-primary-key-in-sqlite
[2] https://sqlite.org/lang_createtable.html#the_primary_key

For the purposes of determining the uniqueness of primary key values, NULL values are considered distinct from all other values, including other NULLs.

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Bug 1987298 - Make moz_openpages_temp.groupId NOT NULL. r?mak

SQLite allows NULL in primary key and allows non-unique primary key if the primary key contains NULL. Stop using NULL to avoid this SQLite feature.

Comment 6

[Masatoshi Kimura [:emk]]

test_providerOpenTabs.js did not catch the bug because memory table is not used until UrlbarProviderOpenTabs.promiseDBPopulated is resolved.

Comment 7

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1908451

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7ed69c9d4691
https://hg.mozilla.org/integration/autoland/rev/664c91195913
Make moz_openpages_temp.groupId NOT NULL. r=mak,places-reviewers,urlbar-reviewers

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/664c91195913

Comment 10

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:emk, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox144 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Users cannot switch to tab even if it is already open.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: The changes are closed in SQL and there are no application logic changes.
• String changes made/needed: No
• Is Android affected?: no

Comment 12

[Masatoshi Kimura [:emk]]

Attachment: Bug 1987298 - Make moz_openpages_temp.groupId NOT NULL. r?mak

SQLite allows NULL in primary key and allows non-unique primary key if the primary key contains NULL. Stop using NULL to avoid this SQLite feature.

Original Revision: https://phabricator.services.mozilla.com/D264089

Comment 13

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/2dd929ee9e41
https://hg.mozilla.org/releases/mozilla-beta/rev/15c03b03a525

Comment 14

[Monica Chiorean (OOO August 21st 5:30PM)]

I was able to reproduce the issue on Win11x64 using FF build 141.0a1(20250604040711).
Verified as fixed on Win11x64 using FF 145.0a1 and 144.0b7.