408470 - Page Style (e.g. "No Style") does not persist across sessions

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Håkan Waara]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b2pre) Gecko/2007121321 Minefield/3.0b3pre

Page style does not persist across sessions. 

STR:

1. Go to any web page with CSS
2. Change the View > Page Style to "No Style"
3. Restart your browser

Actual result:

The web page is restored, but with all the CSS, and not with the style you set.

Expected result:

The web page should be restored with "No Style" on still

Comment 1

[Simon Bünzli]

Attachment: fix

This patch saves the one information bit that's used by the View -> Page Style menu (the title of the currently enabled, named stylesheet(s)).

Comment 2

[Simon Bünzli]

Attachment: alternative fix

This alternative patch just saves the URLs of all disabled stylesheets and in addition to that the state of authorStyleDisabled (for "No Style"). This does however not necessarily restore the state as it can be set through View -> Page Style, as style sheets can be activated through JavaScript in ways the menu can't. Not sure which solution is better though...

Comment 3

[Dietrich Ayala (:dietrich)]

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

i prefer option #1, since it's broader, taking into account script actions as you mentioned, as well as pages that have changed since between restores. it provides a higher likelihood that the restored page will match the user's expectations.

Comment 4

[Simon Bünzli]

Attachment: unbitrotted and with tests

This is pretty much the same patch as you've already reviewed with two differences:
* we now ignore style sheets with a non-screen media type (e.g. for print)
* there's a bunch of tests included

Comment 5

[Dietrich Ayala (:dietrich)]

Comment on attachment 337111 [details] [diff] [review]
unbitrotted and with tests


>+
>+  /**
>+   * determine the title of the currently enabled style sheet (if any)
>+   * and recurse through the frameset if necessary
>+   * @param aContent is a frame reference
>+   */
>+  _getSelectedPageStyle: function sss_getSelectedPageStyle(aContent) {

add @return doc please

r=me

Comment 6

[Simon Bünzli]

Attachment: for check-in (updated to comment #5)

Comment 7

[Reed Loden [:reed]]

http://hg.mozilla.org/mozilla-central/rev/4a01683e50bb