644413 - Scratchpads should be able to restore their context via mode-line

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect, P2)

Description

[Rob Campbell [:rc] (:robcee)]

When loading a .js file into a workspace, it currently defaults to "Content" context. We should make the Workspace respect a mode-line comment at the start of the file to indicate which context to use.

I'm thinking something like:

/* -ws-context: chrome */

initially.

This could also be used to save window state, color-schemes, etc.

Comment 1

[Rob Campbell [:rc] (:robcee)]

I think this is a dupe.

Comment 2

[Anton Kovalyov (:anton)]

I couldn't find any similar bug so I don't think it's a dupe?

Comment 3

[Anton Kovalyov (:anton)]

Attachment: Add support for -sp-context mode-line

In addition to adding support for the -sp-context mode-line I moved file writing function from browser_scratchpad_files.js and into head.js so that I could reuse it.

Comment 4

[Heather Arthur [:harth]]

Comment on attachment 703094 [details] [diff] [review]
Add support for -sp-context mode-line

Review of attachment 703094 [details] [diff] [review]:
-----------------------------------------------------------------

Fix these couple things, and attach the modeline test so I can review that.

::: browser/devtools/scratchpad/scratchpad.js
@@ +64,5 @@
> +    if (ch1 !== "/" || (ch2 !== "*" && ch2 !== "/")) {
> +      return obj;
> +    }
> +
> +    aLine = aLine.slice(2, ch2 === "*" ? aLine.length - 2 : aLine.length);

either comment on what this is doing or do a regex replace(/*/, "") that might be more explicit.

@@ +69,5 @@
> +    aLine.split(",").forEach(function (pair) {
> +      let [key, val] = pair.split(":");
> +
> +      if (key && val) {
> +        obj[key.trim()] = val.trim();  

trailing whitespace here

@@ +692,5 @@
>                                                    aInputStream.available());
>          content = converter.ConvertToUnicode(content);
> +
> +        // Check to see if the first line is a mode-line comment.
> +        let line = content.split("\n")[0].trim();

You already call trim() in the scanModeLine() function, just need to do it in one of these places.

::: browser/devtools/scratchpad/test/browser_scratchpad_files.js
@@ +34,5 @@
>  
> +  createTempFile("fileForBug636725.tmp", gFileContent, function(aStatus, aFile) {
> +    ok(Components.isSuccessCode(aStatus),
> +      "The temporary file was saved successfully");
> +    

trailing whitespace here as well.

Comment 5

[Anton Kovalyov (:anton)]

Attachment: Add support for -sp-context mode-line

Better patch.

Comment 6

[Heather Arthur [:harth]]

Comment on attachment 704137 [details] [diff] [review]
Add support for -sp-context mode-line

Review of attachment 704137 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/scratchpad/test/browser_scratchpad_bug_644413_modeline.js
@@ +38,5 @@
> +
> +  // Test Scratchpad._scanModeLine method.
> +  let obj = gScratchpad._scanModeLine();
> +  is(size(obj), 0, "Mode-line object has no properties");
> +  

more trailing whitespace in this file, just remove before checking in.

::: browser/devtools/scratchpad/test/head.js
@@ -2,4 @@
>  /* Any copyright is dedicated to the Public Domain.
>     http://creativecommons.org/publicdomain/zero/1.0/ */
>  
> -"use strict";

Why'd you get rid of this?

Comment 7

[Anton Kovalyov (:anton)]

Attachment: Add support for -sp-context mode-line

Added back "use strict" and removed trailing spaces. Carrying over r+.

Comment 8

[Anton Kovalyov (:anton)]

https://hg.mozilla.org/integration/fx-team/rev/0b629aad54cc

Comment 9

[Paul Rouget [:paul]]

What happens if a scratchpad with a browser modeline is loaded, but devtools.chrome.enabled pref is off?

I think it should not respect the modeline in this case.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

That's right, otherwise we are opening a big security hole in our tools.

Comment 11

[Paul Rouget [:paul]]

See bug 832880.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/0b629aad54cc