967 bytes, patch
Simon Bünzli: review-
|Details | Diff | Splinter Review|
Created attachment 381212 [details] sessionstore.js - Google maps search for 90210 under Firefox 18.104.22.168 In the 1.9.1 and trunk branches, session data for Google Maps is not saved correctly as such any searches, directions input, etc are not restored when the page is restored. For example, go to maps.google.com and type in the name of a city and wait for the page to load. Close the tab and then reopen the closed tab. In Firefox 22.214.171.124, the page will be restored correctly. In Firefox 3.5b4 (possibly earlier) and later, the main Google Maps page will display and all user inputs are lost. The problem is only on collecting and saving the session data because if a sessionstore.js file created by Firefox 126.96.36.199 is loaded by Minefield the, Google Maps page is correctly restored. Looking at the differences between the sessionstore.js files created under Firefox 188.8.131.52 and Minefield, it appears the frames in Google Maps are not being saved under Shiretoko or Minefield, while they are in Firefox 184.108.40.206. I don't have time to regression range test this at the moment, but I'm leaning towards bug 346337 as a potential cause.
Created attachment 381213 [details] sessionstore.js - Google maps search for 90210 under Minefield Gecko/20090601 Both this and the other attached sessionstore.js were created by starting Firefox/Minefield with a blank page and then going directly to http://maps.google.com and searching for "90210". Notice that in this sessionstore, there are no "children" elements, despite maps.google.com containing two iframes.
reproducible in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080904020533 Minefield/3.1b1pre and bug 346337 got checked in in 2008.09.05, so it can't be that.
likely suspect is bug 450595
Yep, definitely caused by bug 450595. I reverted the fix and the problem went away. I'm guessing a broken Gmail is worse than a non-restoring Google Maps, but there should be some way to have both work correctly.
Is there a reason why the fix to bug 450595 blocks all wyciwyg URLs from being saved instead of those just from Gmail? Is it because checking for specific sites in nsSessionStore would be overly specific? The reason I ask is that the fix to bug 450595 is kind of a throw the baby out with the bath water fix, in that it assumes any site that has a wyciwyg won't restore correctly. Actually if users use the secure (https) version of GMail the page wouldn't restore any way. It would be trivial to restrict the bug 450595 fix to only apply at mail.google.com (or exclude it from applying at maps.google.com), but I'm not sure that's the best method since at that point Firefox would actually be checking for host names in the code. It might be a good idea to have a blacklist for sites the user doesn't want to restore or that don't work reliably. That way fixes like 450595 could be removed without breaking sites.
I found that GMail's wyciwyg URL begins with: wyciwyg://\d+/https://mail.google.com while Google Maps's begins with: wyciwyg://\d+/http://maps.google.com/ So changing the regular expression from the bug 450595 fix from "/^wyciwyg:\/\//" to "/^wyciwyg:\/\/\d+\/https:\/\//" would allow Google Maps to be restored correctly and GMail to be restored reliably. All without having to specifically check for a host name. Would this be an acceptable fix?
Created attachment 381305 [details] [diff] [review] Patch that fixes Google Maps without breaking GMail. This is the change I talked about in comment #6. Would this be considered an "acceptable" fix or is it too specific?
Attachment #381305 - Flags: review?(zeniko)
Comment on attachment 381305 [details] [diff] [review] Patch that fixes Google Maps without breaking GMail. WYCIWYG URLs can be an issue no matter how they were transmitted. I'm not sure how we could best restore a frameset which contains at least some of the dynamically created frames without risking to break any possible web app out there. Guess we need a better plan here, though...
Attachment #381305 - Flags: review?(zeniko) → review-
Would fixing bug 387598 solve the issue here? Google Maps does provide a "link" to the current map, which if used will be correctly restored, but it isn't saved/restored either.
(In reply to comment #9) > Would fixing bug 387598 solve the issue here? Not necessarily, as there's no guarantee for a specific page to be in the bfcache. You'll want to ask a WYCIWYG guru for what might be the best thing to do here.
I don't know who the WYCIWYG gurus are. I found that if I simply strip out the "wyciwyg://\d+/" from the URL, that Google Maps will successfully restore the page. By that I mean change "wyciwyg://0/http://maps.google.com/" to "http://maps.google.com/". Is there a downside to simply converting wyciwyg URLS to regular URLs? That should force them to act as normal requests. Also was bug 450595 supposed to strip out the wyciwyg urls only on a restore? As it is now, they are removed before they are even saved or stored. As such there's no way to get at the URLS via any of the SessionStore API calls.
(In reply to comment #11) > I don't know who the WYCIWYG gurus are. http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/content/html/document/src/nsWyciwygChannel.cpp Maybe jst or bz? Asking on IRC or mozilla.dev.platform might also help... > Also was bug 450595 supposed to strip out the wyciwyg urls only on a restore? No, as there was hardly any point in keeping data around we were never going to use. You should be able to manually insert those URLs during the "sessionstore-state-write" notification to work around the worst part of the issue, though.
Are there any news on fixing this? FYI, i'm successfully using Michael' patch above for almost a year now with absolutely no problems (except for the fact that i have to patch it manually with every Fx update).
Google Maps now seems to restore directions, and load "My places" for me. Tested using: Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0 Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0 Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20120420 Firefox/13.0a2 Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20120423 Firefox/13.0a2 Checked using my normal profile, and a test profile. I'm guessing that Google have changed their code (it seems unlikely that this has been fixed since Fx 7 and no-one noticed). Michael, can you check that Maps works for you without your fix?
Yes it appears to work now. I checked and Google Maps no longer uses http://wyciwyg URLs. Google must have decided to stop using WYCIWYG for whatever reason. As such I'm marking this WORKSFORME.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.