Closed
Bug 767693
Opened 12 years ago
Closed 12 years ago
[New Tab Page] need to save block list when unblocking a site
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 16
People
(Reporter: ttaubert, Assigned: ttaubert)
References
Details
(Whiteboard: [fixed-in-fx-team])
Attachments
(1 file)
2.78 KB,
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
When bug 735987 was fixed I forgot to save the modified block list to disk after unblocking a site. This can lead to the issue described in bug 735987 again when restarting the browser because the 'unblock' modification is lost.
Attachment #636061 -
Flags: review?(jaws)
Comment 1•12 years ago
|
||
Comment on attachment 636061 [details] [diff] [review] patch v1 Review of attachment 636061 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/newtab/browser_newtab_bug735987.js @@ +18,5 @@ > checkGrid("0,99p,1,2,3,4,5,6,7"); > > + NewTabUtils.blockedLinks.resetCache(); > + yield addNewTabPageTab(); > + checkGrid("0,99p,1,2,3,4,5,6,7"); I don't see a |block| between the |simulateDrop| on line 17 and the |resetCache| on line 20, so I'm not sure if this test is actually reproducing the problem that this patch is trying to fix. Shouldn't this be: > yield simulateDrop(1); > checkGrid("0,99p,1,2,3,4,5,6,7"); > NewTabUtils.blockedLinks.block("about:blank#99p"); > NewTabUtils.blockedLinks.resetCache(); > yield addNewTabPageTab(); > checkGrid("0,99p,1,2,3,4,5,6,7");
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Jared Wein [:jaws] from comment #1) > I don't see a |block| between the |simulateDrop| on line 17 and the > |resetCache| on line 20, so I'm not sure if this test is actually > reproducing the problem that this patch is trying to fix. I checked that the test is failing without the patch applied. The block() call from line 14 is important. This should be undone with the simulateDrop() call from line 17. We check this by trying to block again on line 24 and see if we were successful. The only difference here is that I simulated a user restarting the browser by just clearing the BlockedLinks cache and opening a new about:newtab instance. > Shouldn't this be: > > yield simulateDrop(1); > > checkGrid("0,99p,1,2,3,4,5,6,7"); > > NewTabUtils.blockedLinks.block("about:blank#99p"); > > NewTabUtils.blockedLinks.resetCache(); > > yield addNewTabPageTab(); > > checkGrid("0,99p,1,2,3,4,5,6,7"); To explain again, we're checking if unblock() is working when dropping a blocked link onto the grid. block() is definitely working as assured by other tests.
Comment 3•12 years ago
|
||
Comment on attachment 636061 [details] [diff] [review] patch v1 Review of attachment 636061 [details] [diff] [review]: ----------------------------------------------------------------- OK, that makes sense now. Thanks for the clarification.
Attachment #636061 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 4•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/a6aeae2f1514
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 16
Assignee | ||
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a6aeae2f1514
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•