876313 - [New Tab Page] Unpinning a site doesn't update pref

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Geoff Lankow (:darktrojan)]

Using the unpin button on a site does not update browser.newtabpage.pinned, so the site becomes pinned again after a restart.

Comment 1

[Geoff Lankow (:darktrojan)]

I can reproduce this in Firefox 22 but not 21.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Ah, yes. The save() call got accidently removed in bug 855270.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: re-add missing .save() call when unpinning

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/26488f5030f1

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 754364 [details] [diff] [review]
re-add missing .save() call when unpinning

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 855270
User impact if declined: Unpinning links on about:newtab does not work.
Testing completed (on m-c, etc.): Landed on m-c. Will be in tomorrow's Nightly.
Risk to taking this patch (and alternatives if risky): Very low-risk one-line fix.
String or IDL/UUID changes made by this patch: None.

Comment 6

[bhavana bajaj [:bajaj]]

Comment on attachment 754364 [details] [diff] [review]
re-add missing .save() call when unpinning

Good to land on aurora, will wait for a day or two to make sure this is fixed and there are no fallouts in preparation for Beta uplift.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/88684dedad96

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/releases/mozilla-beta/rev/3656aeb1a150

Comment 9

[Simona Badau, Desktop QA, PTO 13-25aug]

The issue is still reproducible if I force Quit Firefox (from the task manager on Windows or from the terminal by using CTRL+C on Ubuntu). Is this expected in any way?

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Sorry I don't understand what you mean? You're saying that you kill Firefox, what did you do before?

Comment 11

[Simona Badau, Desktop QA, PTO 13-25aug]

(In reply to Tim Taubert [:ttaubert] from comment #10)
> Sorry I don't understand what you mean? You're saying that you kill Firefox,
> what did you do before?

Before killing Firefox I unpinned an already pinned page. 

STR on Ubuntu:
Prerequisites: Have some websites pinned
1. Open Firefox from your terminal.
2. Unpin one of the websites.
3. Go to the terminal from which you opened Firefox and kill the process by pressing on the Ctrl+C keys 
4. Using the terminal - open Firefox again.
5. Check the new tab page.

Actual results:
The unpinned web site in step 2 is still pinned.

Comment 12

[Maria Grazia Alastra [:mga]]

Verified fixed on Windows 7, 64 bits, with Firefox 22 beta 4.
User agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0
Build ID: 20130605070403

Comment 13

[:Gavin Sharp [email: [e-mail]]]

(In reply to Simona B [QA] from comment #9)
> The issue is still reproducible if I force Quit Firefox (from the task
> manager on Windows or from the terminal by using CTRL+C on Ubuntu). Is this
> expected in any way?

This is just a side-effect of the fact that prefs aren't flushed to disk until shutdown in the common case (and unclean shutdown means pref changes aren't persisted). Not something we need to worry about too much - this particular set of data isn't so critical that we need to worry about consistency in the face of crashes.

Comment 14

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20130613 Firefox/23.0
Build ID: 20130613004019

User Agent: Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20100101 Firefox/22.0
Build ID: 20130612084701

For, Firefox 22B5 and 23, this bug can be reproduced on ubuntu 32 bit.

Actual Result:-
I can see both unpinned websites and pinned websites for new opened Firefox when I don't use normal close (using forced close), but as Gavin said we don't need to worry about this too much as it is due to prefs settings.

Comment 15

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:24.0) Gecko/20130725 Firefox/24.0
Build ID: 20130725004004
Built from http://hg.mozilla.org/releases/mozilla-aurora/rev/f61ece780449

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:25.0) Gecko/20130725 Firefox/25.0
Build ID: 20130725030212
Built from http://hg.mozilla.org/mozilla-central/rev/a4c1961bf723

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
Build ID: 20130722172257
Built from http://hg.mozilla.org/releases/mozilla-beta/rev/8b103274f2d1

Tested on latest Aurora, beta and nightly using steps given in comment 11, I don't see any issue. After force quit, the site didn't pinned again.

Comment 16

[u279076]

Careful when editing keywords Samvedana to only add/remove the ones you want. Adding back regression and reproducible keywords.