Closed
Bug 614708
Opened 14 years ago
Closed 14 years ago
Calling setTabValue too early causes correct values to be ignored when access early
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
VERIFIED
FIXED
Firefox 4.0b8
People
(Reporter: zpao, Assigned: zpao)
References
Details
Attachments
(1 file, 1 obsolete file)
3.73 KB,
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
See bug 607016 comment 9 for the use case. This probably should have been done as part of bug 590268 but I missed that particular use. This will be slightly datalossy for consumers who set data too early. There isn't really any way around that though. They should either wait for SSTabRestoring or SSTabRestored to set that data.
Assignee | ||
Comment 1•14 years ago
|
||
Fixes the test issues Ian had. It's a bit more correct regardless. In an ideal world we'd check if we would be keeping track of "state" and not allow data to be set on the tab too early. But that's for sessionstore 2.0 :) I don't really feel like writing a test since it's unintentionally covered by the test in 590268. I should be make that test explicitly test this though (listen for TabOpen, call setTabValue see if the test still passes).
Assignee: nobody → paul
Attachment #493145 -
Flags: review?(dietrich)
Comment 2•14 years ago
|
||
Comment on attachment 493145 [details] [diff] [review] Patch v0.1 The code change looks fine. However, SS is too finicky for me to r+ this without the test you suggested.
Attachment #493145 -
Flags: review?(dietrich)
Assignee | ||
Comment 3•14 years ago
|
||
Now with a test! This essentially simulates what Panorama is doing. I added a check that we wipe out the value of "foo" by the time SSTabRestoring is fired.
Attachment #493145 -
Attachment is obsolete: true
Attachment #493719 -
Flags: review?(dietrich)
Comment 4•14 years ago
|
||
This certainly fixes the test I've been having trouble with! My patches are not out of the woods yet (looks like we need bug 615394), but this is definitely a step in the right direction; let's land it.
Updated•14 years ago
|
Attachment #493719 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 5•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/28066e5483fc
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to comment #6) > Is there a way I can verify this bug? The tests are passing. Manually not really.
Updated•13 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•