485482 - Session Restore should handle invalid XPaths better

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[jag (Peter Annema)]

Attachment: Small test case with input element in a <class=text> element.

Build identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3

Build identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.2a1pre) Gecko/20090326 Minefield/3.6a1pre


If a page of HTML tag soup contains something like <class=text><input type="text"> session restore will write out an XPath like: "/HTML/BODY/FORM[@name='bug']/CLASS=TEXT/INPUT", which the restore code doesn't seem to like too much:

Error: uncaught exception: [Exception... "The expression cannot be converted to return the specified type."  code: "52" nsresult: "0x805b0034 (NS_ERROR_DOM_TYPE_ERR)"  location: "file:///Users/jag/Applications/Firefox.app/Contents/MacOS/components/nsSessionStore.js Line: 2627"]

We shouldn't be writing out invalid XPaths, but this could also be handled by catching any errors thrown by the parser and just skipping restoring that control.

Due to the above exception session restore doesn't finish fixing up the form, and every time the page is reloaded (in this tab) the form will be restored from the values as stored in sessionstore.js.

To reproduce, either set browser.sessionstore.privacy_level to 0 (don't forget to reset it when you're done) or put the attached test case on a non https server; have Firefox when it starts show windows and tabs from last time; and then:

1) Load the test case
2) Select "Bad" and type something in all three text boxes
3) Quit Firefox
4) Start Firefox

  - the 2nd and 3rd text boxes are empty; at least the 3rd one shouldn't be

5) Select "Ugly" and type something in the empty text boxes
6) Reload the test case

  - it says "Bad"; should be "Ugly"

7) Select "Ugly" again
8) Hard (shift) reload the test case

  - it says "Bad"; should be "Good"
  - the first text box isn't empty


If you check the Error Console you'll notice the above exception for each reload, indicating that session restore is being triggered when it shouldn't be.

Comment 1

[Simon Bünzli]

Attachment: patch and test

Right, HTML allows for node names which would have been invalid XML (and even valid XPath!), so we might have to escape them in a way XPath allows it.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Simon, should there be a separate bug on making sure the load handler is removed no matter what happens?

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Not blocking, but we'd take a patch w/tests.

Comment 4

[Simon Bünzli]

(In reply to comment #2)
Peter's already filed bug 485483 for that bit of the issue.

Comment 5

[Dietrich Ayala (:dietrich)]

Comment on attachment 369635 [details] [diff] [review]
patch and test

ugh. r=me.

Comment 6

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/9004413301e4

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 369635 [details] [diff] [review]
patch and test

a191=beltzner

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/19fe8295c107

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED on builds: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090505 Minefield/3.6a1pre ID:20090505031205

and

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b5pre) Gecko/20090505 Shiretoko/3.5b5pre ID:20090505031155