361092 - When quitting before all pages have loaded, session saving remembers some pages as about:blank

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

STR:

1. Have a session that includes some db-generated pages that are long or slow to load (say 30 bugzilla pages)
2. Quit
3. Launch Camino
4. Quit again before all pages have loaded
5. Launch Camino again

Actual: Camino loads 30 tabs; pages which had finished loading before quitting in (4) are loaded; pages which didn't finish loading become tabs of about:blank

Expected: Camino remembers the urls of pages which are in the progress of loading when I quit.

I'm not sure how fixable this is (given session history issues like bug 191145 and possibly bug 341895), or whether it merits 1.1 for sure.  The use-case is mostly QA type people, but conceivably this could be an issue for crash-recovery, too.

Comment 1

[froodian (Ian Leue)]

[1:19am] <ardissone> it's kinda an edge case
[1:19am] <ardissone> but it's a huge hole in ss when you hit it
[1:20am] <ardissone> because it's total dataloss

What do we want to do here?  Is this fixable for 1.1?

Comment 2

[Joni Lehto]

I'd like to verify that this bug affects also Firefox 2.0.0.1 on Windows. And when it occurs it can lead to unexpected data loss which can have quite an impact on your working day.

This bug might need to be reclassified, since it affects both Camino and Firefox in the same way and thus is probably Gecko related.

Comment 3

[Chris Lawson (gone)]

The implementations of session saving on Firefox and Camino are almost completely different, so it's unlikely that one bug covering both would be useful at all. That both implementations happen to have a similar bug does not mean (particularly in this case) that the same code is responsible for both bugs.

cl

Comment 4

[Joni Lehto]

Should I make a similar report of the Firefox bug then?

Comment 5

[Stuart Morgan]

If there isn't already a bug for Firefox, then yes.

Comment 6

[Joni Lehto]

I made a bug report of the similar Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=367605. If anyone fixes this for Camino, please check if your fix could be used to help fixing the Firefox bug also.

Comment 7

[Stuart Morgan]

We could fix this by having each browser remember the uri it's been told to load, and refer to that. In general that's probably not the right behavior, but we could perhaps special-case about:blank, which would cover this case and hopefully not cause too much confusion.

Comment 8

[captain]

I hope someone can fix this before Camino 1.1!  What ardissone notes is true: this is a *total data loss* problem within an ostensibly *data saving* feature.  :-P

I'm sorry I can't fix this, but I have noticed that whenever a new window is opened in Camino the URL doesn't actually appear in the top window until *after* the page is loaded, but it does show up in the bottom line, showing that it is attempting to load.  Perhaps there could be two entries into whatever data structure is being used to hold the data for a restore, and if the "loading" doesn't match the "loaded" and the "loaded" equals about:blank, then re-load the "loading" URL.

Comment 9

[Stuart Morgan]

That's essentially what I was suggesting in comment 7 (except that there's no need to store both since the decision can be made at storage time rather than restore time).

Comment 10

[Stuart Morgan]

Attachment: fix

Implements comment 7.

Comment 11

[Sean Murphy]

Comment on attachment 256380 [details] [diff] [review]
fix

The patch looks great and fixes the issue; r=me.  I also ensured that purposely blank pages (ones that aren't loading anything) are saved and restored properly even with the adjusted technique, which I consider expected behavior.

I didn't notice anything in nsIWebNavigation or similar classes in the Core API which offers a way to obtain a loading URL, so I agree that you took the best approach.

I noticed a few compiler warnings when building the SessionManager code for this patch.  I filed bug 371838 and attached a simple fix.

Comment 12

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 256380 [details] [diff] [review]
fix

sr=pink

Comment 13

[Stuart Morgan]

Checked in on trunk and MOZILLA_1_8_BRANCH.