1168084 - View source preferences should be persisted

Status: VERIFIED FIXED

(Toolkit :: View Source, defect)

Description

[Alice0775 White]

Steps To Reproduce:
1. Open Web page
2. Ctrl+U open View Page Source
3. Right Click and Choose "Wrap Long Line"
4. Ctrl+w close the View Page Source
5. Repeat Step 2

Actual Results:
State of the "Wrap Long Line" is reset

Expected results:

State of the "Wrap Long Line" should be persisted

Comment 1

[Tim Nguyen :ntim]

[Tracking Requested - why for this release]: Regression in view-source functionality

This also affects the Syntax highlighting setting.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a83b048f465c

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: bz://1168084/jryans

/r/9407 - Bug 1168084 - Persist view source tab options. r=mconley

Pull down this commit:

hg pull -r e18b9a9c3bb3b77cf917cb268fa35bfd267ffbc3 https://reviewboard-hg.mozilla.org/gecko/

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Fixed tests:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b6f132e2c94

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8610929 [details]
MozReview Request: bz://1168084/jryans

/r/9407 - Bug 1168084 - Persist view source tab options. r=mconley

Pull down this commit:

hg pull -r b21067d7cdc1dd7d60d6c5cab56dc27c4ab9f8ec https://reviewboard-hg.mozilla.org/gecko/

Comment 6

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Adding a tracking flag for Firefox41. Another regression in View Source component.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

https://reviewboard.mozilla.org/r/9405/#review8569

This looks good! Just one minor suggestion for the tests.

::: toolkit/components/viewsource/test/browser/browser_viewsourceprefs.js:44
(Diff revision 2)
> +  let prefReady = waitForStoreWrapping(win);

Instead of wrapping the functions like this, you can use Preferences.jsm to observe the pref. That'll also test to make sure that the pref was actually updated.

Example:

```
Cu.import("resource://gre/modules/PromiseUtils.jsm");
Cu.import("resource://gre/modules/Preferences.jsm");

// ...

function waitForStorePref() {
  let deferred = PromiseUtils.defer();
  let observer = () => {
    Preferences.ignore("view_source.wrap_long_lines", observer);
    deferred.resolve();
  };
  
  Preferences.observe("view_source.wrap_long_lines", observer);
  return deferred.promise;
}
```

There are possibly better ways of structuring that - but I think I'd prefer this over patching the viewSourceChrome object.

::: toolkit/components/viewsource/test/browser/browser_viewsourceprefs.js:117
(Diff revision 2)
> +function waitForStoreWrapping(win) {
> +  return new Promise(resolve => {
> +    let storeWrapping = win.viewSourceChrome.storeWrapping;
> +    win.viewSourceChrome.storeWrapping = state => {
> +      storeWrapping(state);
> +      win.viewSourceChrome.storeWrapping = storeWrapping;
> +      resolve();
> +    };
> +  });
> +}
> +
> +function waitForStoreSyntaxHighlighting(win) {
> +  return new Promise(resolve => {
> +    let storeSyntaxHighlighting = win.viewSourceChrome.storeSyntaxHighlighting;
> +    win.viewSourceChrome.storeSyntaxHighlighting = state => {
> +      storeSyntaxHighlighting(state);
> +      win.viewSourceChrome.storeSyntaxHighlighting = storeSyntaxHighlighting;
> +      resolve();
> +    };
> +  });
> +}

See my comment above about using Preferences.jsm to observe that the preference was actually updated.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1168084 - Persist view source tab options. r=mconley

Bug 1168084 - Persist view source tab options. r=mconley

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8613710 [details]
MozReview Request: Bug 1168084 - Persist view source tab options. r=mconley

https://reviewboard.mozilla.org/r/9407/#review8597

Looks good! Thanks jryans!

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8ff425fe04e

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8ff42f8c288c

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ff42f8c288c

Comment 14

[Rakibul Islam Ratul]

I have reproduced this bug in Nightly 41.0a1(Build ID: 20150525030205)(Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0) with instruction from comment 0

Verified as fixed now on latest nightly Edition 43.0a1 (Build ID: 20150901030226) 

Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0

[Bugday-20150902]

Comment 15

[Rakibul Islam Ratul]

Verified as fixed on beta 41.0b6 (Build ID: 20150831172306) 

Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0

Comment 16

[Meraj Kazi]

I have reproduced this bug in Nightly 41.0a1 ( Build ID  20150525030205) 
Mozilla/5.0 (X11; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0


Verified as fixed on beta 41.0b6 (Build ID  20150831172306) 
Mozilla/5.0 (X11; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0

[Bugday-20150902]