Closed Bug 597901 Opened 14 years ago Closed 14 years ago

Throttled/cascade-loaded tabs have permanent empty progress bar until force-loaded by clicking on them

Categories

(Firefox :: Session Restore, defect)

Product:
Firefox
Component:
Session Restore
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 4.0b7
Tracking Flags:
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: joe, Assigned: zpao)

References

Details

Attachments

(1 file, 1 obsolete file)

Patch v0.2
(deleted), patch
Gavin
: review+
Details | Diff | Splinter Review
When you restore your session, certain tabs aren't loaded immediately (on purpose, thanks to bug 586068). However, these not-yet-loaded tabs permanently have an empty progress bar on them (which fills in and then disappears once it's force loaded by clicking on the tab).
Summary: Throttled/cascade-loaded tab has permanent empty progress bar until force-loaded by clicking on them → Throttled/cascade-loaded tabs have permanent empty progress bar until force-loaded by clicking on them
Assignee: nobody → margaret.leibovic
Component: Tabbed Browser → Session Restore
QA Contact: tabbed.browser → session.restore
OS: Mac OS X → All
Hardware: x86 → All
Attached patch Patch v0.1 (obsolete) (deleted) — — Splinter Review
Don't setAttribute("busy", true) until we're actually busy.
Assignee: margaret.leibovic → paul
Status: NEW → ASSIGNED
Attachment #476851 - Flags: review?(dietrich)
Comment on attachment 476851 [details] [diff] [review] Patch v0.1 r=me
Attachment #476851 - Flags: review?(dietrich) → review+
Is it necessary to setAttribute("busy") at all? gotoIndex will trigger that on its own via progress notifications and such, won't it? I imagine we did it before because the natural process might take some time, but that should be less of a problem now that we're not restoring all at once, I hope?
Current behaviour is confusing. Wouldn't hold feature freeze for it, but let's get it in anyhow?
blocking2.0: --- → betaN+
(In reply to comment #3) > Is it necessary to setAttribute("busy") at all? gotoIndex will trigger that on > its own via progress notifications and such, won't it? I imagine we did it > before because the natural process might take some time, but that should be > less of a problem now that we're not restoring all at once, I hope? Yea it is set automatically (at least, it works locally). I'm not actually sure of this history behind setting it ourselves. What you imagine sounds reasonable though. I'm fine with taking it out completely. Would you review?
Comment on attachment 476851 [details] [diff] [review] Patch v0.1 (note that it also would have been incorrect to move that line without also moving the call to updateIcon, but bug 597673 makes that point moot)
Attachment #476851 - Flags: approval2.0?
Attachment #476851 - Attachment is obsolete: true
Attached patch Patch v0.2 (deleted) — — Splinter Review
Just the removal of setAttribute("busy", true);
Attachment #478076 - Flags: review?(gavin.sharp)
Attachment #478076 - Flags: review?(gavin.sharp) → review+
Important for cascading session restore.
blocking2.0: betaN+ → beta7+
Pushed http://hg.mozilla.org/mozilla-central/rev/a2d25ef0e84d
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b7
Version: unspecified → Trunk
Flags: in-litmus?(anthony.s.hughes)
Juan set this as in-litmus? back when we had a progress bar. Now we use a 2-state spinner. Juan, do you think this still warrants a testcase?
Status: RESOLVED → VERIFIED
If that check isn't part of any related test, I'd like to see one in litmus.
Additional check added to existing Litmus test: https://litmus.mozilla.org/show_test.cgi?id=13860
Flags: in-litmus?(anthony.s.hughes) → in-litmus+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: