1067325 - Add an option to view html source in a tab

Status: RESOLVED FIXED

(Toolkit :: View Source, defect, P1)

Description

[Pascal Chevrel:pascalc]

It would be nice to be able to define that view-source is opened in a tab and not in a separate window. That allows reducing clutter in your window manager and that also allows keeping the source code of a page next to the tab of the actual page it refers to. If you open the source for 3 pages to compare the generated html (comparing a page on trunk/stage/production servers for example), you want the source to be visually close to the tab it represents.

This is one of the reasons why I still use the Web Developper extension which has this option, to be able to keep view-source next to the page and not in a different window.

Thanks.

Comment 1

[Paul Rouget [:paul]]

I believe this should be the default behavior. Basically, Ctrl-u should just open a new tab with view-source:$URL. We would lose the the context menu and the menubar, but I don't think it's very important.

Comment 2

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Assigning P1 priority for some devedition-40 bugs. 

Filter on '148DD49E-D162-440B-AE28-E22632FC20AC'

Comment 3

[Brian Grinstead [:bgrins]]

Moving to Firefox::Menus since it looks like this is talking about changing the context menu functionality

Comment 4

[Paul Rouget [:paul]]

Why is it just the context menu? What about the toolbar menuitem and the keyboard shortcut?

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Paul Rouget [:paul] from comment #4)
> Why is it just the context menu? What about the toolbar menuitem and the
> keyboard shortcut?

I assumed this bug was about all of those things

Comment 6

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

(In reply to Brian Grinstead [:bgrins] from comment #5)
> (In reply to Paul Rouget [:paul] from comment #4)
> > Why is it just the context menu? What about the toolbar menuitem and the
> > keyboard shortcut?
> 
> I assumed this bug was about all of those things

Yes, I would intend to update all of them.

Comment 7

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

As bug 103622 notes, simply moving to a tab would drop some features from the Edit and View menus:

* Edit -> Go To Line
* View -> Wrap Long Lines
* View -> Syntax Highlighting

One way to regain some features would be for View Source to use the DevTools source editor (CodeMirror), but that's likely follow up work.

We could potentially bind shortcuts to these existing features with menu items, or just drop them for now.

Comment 8

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

(In reply to J. Ryan Stinnett [:jryans] (on PTO Apr. 13 - Apr. 17) from comment #7)
> We could potentially bind shortcuts to these existing features with menu
> items, or just drop them for now.

We could potentially bind shortcuts to these existing features **without** menu items: as in, the shortcut would do something, but there would be no visible option in the menu bar.

Comment 9

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

Hey - note that while fixing bug 1025146, I was planning on cleaning up some of the View Source interfaces that we use. I just found out about this bug today - jryans, is it possible for us to sync up at some point to make sure we don't step on each others toes, or duplicate effort?

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

For the view sources interfaces in the devtools itself (viewing source in debugger, styleeditor, scratchpad), bug 1134778 is consolidating those, and for directly using the `view-source:${url}` protocol, it's still based off of gViewSourceUtils, FWIW

Comment 11

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

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #9)
> Hey - note that while fixing bug 1025146, I was planning on cleaning up some
> of the View Source interfaces that we use. I just found out about this bug
> today - jryans, is it possible for us to sync up at some point to make sure
> we don't step on each others toes, or duplicate effort?

Sure!  However, I will be out on PTO next week, so my first day back is Apr. 20.

What sort of changes are you planning?  Are features removed?  If you are just shifting things around, but features remain the same, I'd say just go for it, especially if you're working on this before I return.

The patch currently in bug 1025146 seems fine to me.  If you've got more complex cleanup plans, please let me know!

Comment 12

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

At the moment, Mike's patches in bug 1025146 seems helpful for this work and touch the same APIs.  I'll wait a bit in hopes that it will land soon.

Comment 13

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

jryans and I talked today. I'm splitting a good chunk of work out into a frame script that, hopefully, can be reused in the tab implementation of view source. (toolkit/components/viewsource/content/viewSource.js, however, should probably not be reused...).

Comment 14

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

Attachment: MozReview Request: bz://1067325/jryans

/r/8923 - Bug 1067325 - Fix line endings in several browser / toolkit files. r=mconley
/r/8925 - Bug 1067325 - Extract view source core to support tabs. r=mconley
/r/8927 - Bug 1067325 - Update charset when history is disabled. r=mconley
/r/8929 - Bug 1067325 - Selection source in tab via frame scripts. r=mconley
/r/8931 - Bug 1067325 - MathML source in tab. r=mconley
/r/8933 - Bug 1067325 - Control syntax highlighting via a class. r=hsivonen
/r/8935 - Bug 1067325 - Context menu actions for view source tabs. r=mconley
/r/8937 - Bug 1067325 - Allow DevTools to use view source in tab. r=jsantell

Pull down these commits:

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

Comment 15

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

The current patch set gives a version of view source in a tab which supports the extra menu options, such as:

* Go To Line
* Wrap Long Lines
* Syntax Highlighting

They can be toggled from the context menu.

All variations of view source are supported, including:

* View Page Source
* View Selection Source
* View MathML Source
* View Frame Source

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

Comment 16

[Jordan Santell [:jsantell] [@jsantell]]

https://reviewboard.mozilla.org/r/8937/#review7597

::: browser/devtools/webconsole/hudservice.js:438
(Diff revision 1)
> +    // a tab, if enabled.

I believe webconsole uses the source-utils files as well, can't we just link them here, rather than dupe the code?

Comment 17

[Jordan Santell [:jsantell] [@jsantell]]

https://reviewboard.mozilla.org/r/8937/#review7599

Ship It!

Comment 18

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

Looks good, although I'd see if webconsole already is using source-utils files and just use that rather than copying the logic there as well

Comment 19

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

https://reviewboard.mozilla.org/r/8937/#review7601

> I believe webconsole uses the source-utils files as well, can't we just link them here, rather than dupe the code?

It does use it, but only for the scratchpad case currently.  I tried to share this new code from source utils at first, but it did not really help the web console case because most of source utils assumes a toolbox exists, while `hudservice.js` supports the Browser Console case where there is no toolbox.

Comment 20

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

Fixed a small issue:

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

Comment 21

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

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

/r/8923 - Bug 1067325 - Fix line endings in several browser / toolkit files. r=mconley
/r/8925 - Bug 1067325 - Extract view source core to support tabs. r=mconley
/r/8927 - Bug 1067325 - Update charset when history is disabled. r=mconley
/r/8929 - Bug 1067325 - Selection source in tab via frame scripts. r=mconley
/r/8931 - Bug 1067325 - MathML source in tab. r=mconley
/r/8933 - Bug 1067325 - Control syntax highlighting via a class. r=hsivonen
/r/8935 - Bug 1067325 - Context menu actions for view source tabs. r=mconley
/r/8937 - Bug 1067325 - Allow DevTools to use view source in tab. r=jsantell

Pull down these commits:

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

Comment 22

[Jordan Santell [:jsantell] [@jsantell]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #19)
> https://reviewboard.mozilla.org/r/8937/#review7601
> 
> > I believe webconsole uses the source-utils files as well, can't we just link them here, rather than dupe the code?
> 
> It does use it, but only for the scratchpad case currently.  I tried to
> share this new code from source utils at first, but it did not really help
> the web console case because most of source utils assumes a toolbox exists,
> while `hudservice.js` supports the Browser Console case where there is no
> toolbox.

Ah well that gets messy -- would it make sense to open a bug for a source-utils `viewSource` that works in the browser toolbox case? We should keep the forked logic stuffed inside of source-utils so that any other environment using these utilities don't have to think about edge cases like no toolbox, no related browser window, etc

Comment 23

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

https://reviewboard.mozilla.org/r/8923/#review7611

Ship It!

Comment 24

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

Fixed some DevTools tests:

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

Comment 25

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

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

/r/8923 - Bug 1067325 - Fix line endings in several browser / toolkit files. r=mconley
/r/8925 - Bug 1067325 - Extract view source core to support tabs. r=mconley
/r/8927 - Bug 1067325 - Update charset when history is disabled. r=mconley
/r/8929 - Bug 1067325 - Selection source in tab via frame scripts. r=mconley
/r/8931 - Bug 1067325 - MathML source in tab. r=mconley
/r/8933 - Bug 1067325 - Control syntax highlighting via a class. r=hsivonen
/r/8935 - Bug 1067325 - Context menu actions for view source tabs. r=mconley
/r/8937 - Bug 1067325 - Allow DevTools to use view source in tab. r=jsantell
/r/8961 - Bug 1067325 - DevTools test fixes for view source in tab. r=pbrosset

Pull down these commits:

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

Comment 26

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

https://reviewboard.mozilla.org/r/8925/#review7647

::: toolkit/components/viewsource/ViewSourceBrowser.jsm:19
(Diff revision 3)
> + * ViewSourceBrowser manages for the view source <browser> from the chrome side.

Nit - "manages for the" -> "manages the"

::: toolkit/components/viewsource/ViewSourceBrowser.jsm:25
(Diff revision 3)
> + * The page script takes care of load the companion frame script.

s/load/loading.

We might want to file a follow-up so that both the viewSource.xul and browser.js in the newly created tab get the framescript loaded on demand somewhere in this module, just to centralize.

::: toolkit/components/viewsource/ViewSourceBrowser.jsm:85
(Diff revision 3)
> +        // Unhandled
> +        return false;

So we return false in the event that we don't handle the message?

Hrm. That sounds easy to forget / kinda foot-gunny.

Can we make it so that subclasses should check the messages Array of the parent class instead?

::: toolkit/components/viewsource/content/viewSource.js:130
(Diff revision 3)
> +    if (ViewSourceBrowser.prototype.receiveMessage.call(this, message)) {
> +      // Handled in super class
> +      return;
> +    }

Here's where I suggest we check the parents list of supported messages, instead of calling receiveMessage.

::: toolkit/components/viewsource/content/viewSource.js:305
(Diff revision 3)
> -    gBrowser.droppedLinkHandler = function (event, url, name) {
> -      ViewSourceChrome.loadURL(url);
> +    this.browser.droppedLinkHandler = (event, url, name) => {
> +      this.loadURL(url);
>        event.preventDefault();
>      };

Incidentally, do we know if this is even necessary now that bug 1158377 has landed? I'm pretty sure the patch I landed there should work with and without e10s... so this might be superfluous.

Not really a thing you need to be concerned about a guess, but might be worth filing a follow-up about.

::: toolkit/components/viewsource/content/viewSource-content.js:101
(Diff revision 3)
> +    if (!this.isViewSource) {
> +      return;
> +    }

Out of curiosity, in what scenarios is this necessary?

::: toolkit/components/viewsource/content/viewSource-content.js:135
(Diff revision 3)
> +    if (!this.isViewSource) {
> +      return;
> +    }

Same as above - in what scenarios is this necessary?

Comment 27

[Patrick Brosset <:pbro>]

https://reviewboard.mozilla.org/r/8961/#review7653

Nice!

Comment 28

[Patrick Brosset <:pbro>]

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

Ok, I marked the commit you flagged me for review on as "ship it" in mozreview, but that somehow didn't make it back to bugzilla. So consider this an R+ for that part.

Comment 29

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

https://reviewboard.mozilla.org/r/8927/#review7655

::: toolkit/components/viewsource/content/viewSource.js:457
(Diff revision 3)
> -      if (this.historyEnabled) {
> +      if (!this.historyEnabled) {

Whoops!

Comment 30

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

https://reviewboard.mozilla.org/r/8925/#review7659

> s/load/loading.
> 
> We might want to file a follow-up so that both the viewSource.xul and browser.js in the newly created tab get the framescript loaded on demand somewhere in this module, just to centralize.

Okay, filed bug 1166332.

Comment 31

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

https://reviewboard.mozilla.org/r/8935/#review7661

::: toolkit/components/viewsource/test/browser/browser_viewsourceprefs.js:120
(Diff revision 3)
>      let spans = content.document.getElementsByTagName("span");
>      return Array.some(spans, (span) => {
> -      return span.className != "";
> -    });
> +      let style = content.getComputedStyle(span, null);
> +      return style.getPropertyValue("color") !== "rgb(0, 0, 0)";
> -  });
> +    });

Indentation is busted here.

::: toolkit/components/viewsource/test/browser/browser_viewsourceprefs.js:125
(Diff revision 3)
>    is(highlighted, expected, "Syntax highlighting " + (expected ? "on" : "off"));

Where is "highlighted" being defined now?

Comment 32

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

https://reviewboard.mozilla.org/r/8929/#review7657

::: toolkit/components/viewsource/content/viewSource-content.js:198
(Diff revision 3)
> +    return docShell.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
> +                   .getInterface(Components.interfaces.nsIWebBrowserFind);

You can use Ci in here.

Comment 33

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

https://reviewboard.mozilla.org/r/8931/#review7663

I won't lie, some of that old fragment code from viewPartialSource is straight-up terrifying.

But, well, if it's worked so far, that's good enough for me.

Great job with this.

Comment 34

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

https://reviewboard.mozilla.org/r/8925/#review7665

> Incidentally, do we know if this is even necessary now that bug 1158377 has landed? I'm pretty sure the patch I landed there should work with and without e10s... so this might be superfluous.
> 
> Not really a thing you need to be concerned about a guess, but might be worth filing a follow-up about.

You're right, it's not needed anymore.  I've removed this block and the similar one in `viewPartialSource.js`.

Comment 35

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

https://reviewboard.mozilla.org/r/8935/#review7669

> Where is "highlighted" being defined now?

Hmm, odd...  I did not mean to delete the `let highlighted` line at all.  It's back now, which makes the identation make sense too.

Comment 36

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

https://reviewboard.mozilla.org/r/8925/#review7671

> So we return false in the event that we don't handle the message?
> 
> Hrm. That sounds easy to forget / kinda foot-gunny.
> 
> Can we make it so that subclasses should check the messages Array of the parent class instead?

Okay, I changed to have the subclass repeat the case blocks for super class messages, as discussed on IRC.

Comment 37

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

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #22)
> (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #19)
> > https://reviewboard.mozilla.org/r/8937/#review7601
> > 
> > > I believe webconsole uses the source-utils files as well, can't we just link them here, rather than dupe the code?
> > 
> > It does use it, but only for the scratchpad case currently.  I tried to
> > share this new code from source utils at first, but it did not really help
> > the web console case because most of source utils assumes a toolbox exists,
> > while `hudservice.js` supports the Browser Console case where there is no
> > toolbox.
> 
> Ah well that gets messy -- would it make sense to open a bug for a
> source-utils `viewSource` that works in the browser toolbox case? We should
> keep the forked logic stuffed inside of source-utils so that any other
> environment using these utilities don't have to think about edge cases like
> no toolbox, no related browser window, etc

Makes sense, I filed bug 1166391 for this.

Comment 38

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

Review nits fixed, and another DevTools test fix:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=38b5bc001bfe

Comment 39

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

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

/r/8923 - Bug 1067325 - Fix line endings in several browser / toolkit files. r=mconley
/r/8925 - Bug 1067325 - Extract view source core to support tabs. r=mconley
/r/8927 - Bug 1067325 - Update charset when history is disabled. r=mconley
/r/8929 - Bug 1067325 - Selection source in tab via frame scripts. r=mconley
/r/8931 - Bug 1067325 - MathML source in tab. r=mconley
/r/8933 - Bug 1067325 - Control syntax highlighting via a class. r=hsivonen
/r/8935 - Bug 1067325 - Context menu actions for view source tabs. r=mconley
/r/8937 - Bug 1067325 - Allow DevTools to use view source in tab. r=jsantell
/r/8961 - Bug 1067325 - DevTools test fixes for view source in tab. r=pbrosset

Pull down these commits:

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

Comment 40

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

Patrick, the DevTools test patch you reviewed now also contains a webbrowser.js change that fixed the modified browser_computedview_style-editor-link.js test in the e10s suite.  Probably worth another review.

Comment 41

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

https://reviewboard.mozilla.org/r/8935/#review7693

Ship It!

Comment 42

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

https://reviewboard.mozilla.org/r/8925/#review7699

Ship It!

Comment 43

[Patrick Brosset <:pbro>]

https://reviewboard.mozilla.org/r/8961/#review7737

::: toolkit/devtools/server/actors/webbrowser.js:1318
(Diff revision 2)
> +    for (let sheetActor of this._styleSheetActors.values()) {

I'm unsure about this change. Will this be executed when the location changes (it used to be in `_windowReady` before)?
I guess if you tested that connecting again to the same app via WebIDE after the toolbox was closed still works, then fine, but I don't know this code enough to know if that change will be a problem.

Comment 44

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

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

https://reviewboard.mozilla.org/r/8921/#review7747

I've r+'d my bits. Thanks jryans!

Comment 45

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

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #43)
> https://reviewboard.mozilla.org/r/8961/#review7737
> 
> ::: toolkit/devtools/server/actors/webbrowser.js:1318
> (Diff revision 2)
> > +    for (let sheetActor of this._styleSheetActors.values()) {
> 
> I'm unsure about this change. Will this be executed when the location
> changes (it used to be in `_windowReady` before)?
> I guess if you tested that connecting again to the same app via WebIDE after
> the toolbox was closed still works, then fine, but I don't know this code
> enough to know if that change will be a problem.

No, it's no longer executed when the location changes.  This means the set of style sheet actors is now held alive until we detach from the tab (typically this is on toolbox close).

I did a few tests with both web sites and apps on the simulator, performing both location changes and cycling the toolbox open and closed.  I did not see any errors, and the inspector and style editor appeared to reference the correct style sheets.

Comment 46

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

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

Cameron, would you be able to review the patch "Control syntax highlighting via a class.", or would you know someone who can?

We're hoping to land this work quite soon, and I can't seem to reach hsivonen at the moment.

Comment 47

[Henri Sivonen (:hsivonen)]

https://reviewboard.mozilla.org/r/8933/#review7901

r+ for the parser/style sheet bits.

Comment 48

[Henri Sivonen (:hsivonen)]

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

Trying to manually sync the flags with Review Board.

Comment 49

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

Comment on attachment 8607060 [details]
MozReview Request: bz://1067325/jryans

Cameron's review no longer needed.

Comment 50

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/da3c181e9fae
https://hg.mozilla.org/integration/fx-team/rev/f32ee4ca1af4
https://hg.mozilla.org/integration/fx-team/rev/038f30351bb3
https://hg.mozilla.org/integration/fx-team/rev/a6c91e3114ca
https://hg.mozilla.org/integration/fx-team/rev/0dbdd6fc4701
https://hg.mozilla.org/integration/fx-team/rev/2f9d54c153ed
https://hg.mozilla.org/integration/fx-team/rev/03243db46bf0
https://hg.mozilla.org/integration/fx-team/rev/71aa79b01224
https://hg.mozilla.org/integration/fx-team/rev/a5e245f216b7

Comment 51

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/fb5c3e7828e4
https://hg.mozilla.org/integration/fx-team/rev/57227938bb6a
https://hg.mozilla.org/integration/fx-team/rev/d1829d0c6e38
https://hg.mozilla.org/integration/fx-team/rev/d766cf1d3fb4
https://hg.mozilla.org/integration/fx-team/rev/8d3952e2deb4
https://hg.mozilla.org/integration/fx-team/rev/12d5086eebb3
https://hg.mozilla.org/integration/fx-team/rev/3069fa9c2ffd
https://hg.mozilla.org/integration/fx-team/rev/0b01db3f7238

Comment 52

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

Also part of backout:

https://hg.mozilla.org/integration/fx-team/rev/24b328fec94a

I messed up the bug number in that one... :(

Comment 53

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

Fixed broken reftests:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c0e9a907d66

Comment 54

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #53)
> Fixed broken reftests:
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c0e9a907d66

Works great!

Comment 55

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

Fixed broken mochitests:

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

Let's try landing again.

Comment 56

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/650b90c446dc
https://hg.mozilla.org/integration/fx-team/rev/197f69574d75
https://hg.mozilla.org/integration/fx-team/rev/5caf0a502003
https://hg.mozilla.org/integration/fx-team/rev/da3d874d37e7
https://hg.mozilla.org/integration/fx-team/rev/0e91a598b99e
https://hg.mozilla.org/integration/fx-team/rev/0f9ad74c8a72
https://hg.mozilla.org/integration/fx-team/rev/2f97164b55fd
https://hg.mozilla.org/integration/fx-team/rev/64916196381b
https://hg.mozilla.org/integration/fx-team/rev/08dfcb03f55b

Comment 57

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/650b90c446dc
https://hg.mozilla.org/mozilla-central/rev/197f69574d75
https://hg.mozilla.org/mozilla-central/rev/5caf0a502003
https://hg.mozilla.org/mozilla-central/rev/da3d874d37e7
https://hg.mozilla.org/mozilla-central/rev/0e91a598b99e
https://hg.mozilla.org/mozilla-central/rev/0f9ad74c8a72
https://hg.mozilla.org/mozilla-central/rev/2f97164b55fd
https://hg.mozilla.org/mozilla-central/rev/64916196381b
https://hg.mozilla.org/mozilla-central/rev/08dfcb03f55b

Comment 58

[Sylvestre Ledru [:Sylvestre]]

Release Note Request (optional, but appreciated)
[Why is this notable]: Nice change
[Suggested wording]: Open view source in a new tab (is it the case by default?)
[Links (documentation, blog post, etc)]: Is there any?

Comment 59

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

(In reply to Sylvestre Ledru [:sylvestre] from comment #58)
> [Suggested wording]: Open view source in a new tab (is it the case by
> default?)

Yes, it's enabled by default.  You can return to the previous window mode by setting the pref  "view_source.tab" to false.

> [Links (documentation, blog post, etc)]: Is there any?

There is not any for the moment.  I'll set dev-doc-needed to update any that may exist.

Comment 61

[Fernando García Gómez, stripTM]

There has been a regression with regards to the current functionality. The view-source tab is now linked to the original one. When reloading the source code tab in the new version, the changes are not displayed, we need to go back to the source tab, then reload it, and then go to the view-source tab and finally reload again.

Example:
 -Visit http://striptm.com/ejemplos/date.php
 -Ctrl+u
 -Ctrl+r (in view-source tab)

Expected:
As in the previous version, the code content must be updated.

Comment 62

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

(In reply to Fernando García Gómez, stripTM from comment #61)
> There has been a regression with regards to the current functionality. The
> view-source tab is now linked to the original one. When reloading the source
> code tab in the new version, the changes are not displayed, we need to go
> back to the source tab, then reload it, and then go to the view-source tab
> and finally reload again.
> 
> Example:
>  -Visit http://striptm.com/ejemplos/date.php
>  -Ctrl+u
>  -Ctrl+r (in view-source tab)
> 
> Expected:
> As in the previous version, the code content must be updated.

Regressions should be filed in a new bug.  In this case, your issue is already filed as bug 1167797.

Comment 63

[Boris Zbarsky [:bzbarsky]]

Is the plan to keep the non-tab option around, in general?  Because usually I want to be looking at both the source _and_ the page at the same time, and this is obviously harder if view-source is another tab.

Comment 64

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

(In reply to Not doing reviews right now from comment #63)
> Is the plan to keep the non-tab option around, in general?  Because usually
> I want to be looking at both the source _and_ the page at the same time, and
> this is obviously harder if view-source is another tab.

Yes, the window mode will remain usable by flipping "view_source.tab" to false.

I have no plans to remove it, and I would imagine it's desired for non-browser applications anyway.

Eventually, we'd like to the "view source" operation to open the source in the DevTools in some way, which would likely show the source somewhere in the toolbox and could be docked into the same tab as the content.  But, there are still many UX questions and such to resolve before getting there, as we don't yet know how we want something like that to work in detail.

Comment 65

[Boris Zbarsky [:bzbarsky]]

> we'd like to the "view source" operation to open the source in the DevTools in some way,

Please make sure to test that this works reasonably when the source is a few megabytes of HTML before making such a change...

Comment 66

[Will Bamberg [:wbamberg]]

I found an old page on this, moved it under Tools and updated it. I'm not sure if all the stuff under "Limitations" still applies, but it seemed plausible enough.

https://developer.mozilla.org/en-US/docs/Tools/View_source

Comment 67

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

(In reply to Will Bamberg [:wbamberg] from comment #66)
> I found an old page on this, moved it under Tools and updated it. I'm not
> sure if all the stuff under "Limitations" still applies, but it seemed
> plausible enough.
> 
> https://developer.mozilla.org/en-US/docs/Tools/View_source

Great, this is good to have!

I think we should also document the additional options, since they are very non-discoverable today:

Inside the view source tab, the context menu offers Go to Line, Wrap Long Lines, and Toggle Syntax Highlighting.  Also, Go to Line is mapped to Ctrl-Opt-L (Mac) / Alt-Shift-L (Win / Linux).  There's bug 1171315 and bug 1171316 to improve this in the future, but that's what we've got for now.

Comment 68

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

(In reply to Will Bamberg [:wbamberg] from comment #66)
> I found an old page on this, moved it under Tools and updated it. I'm not
> sure if all the stuff under "Limitations" still applies, but it seemed
> plausible enough.
> 
> https://developer.mozilla.org/en-US/docs/Tools/View_source

Also, there are a few more features, possibly worth mentioning:

# Selection Source

You can select any region of the page, then use the context menu to choose "View Selection Source" and see just that region's source.

# MathML Source

On a page with MathML[1], you can "View MathML Source" from the context menu. 

[1]: https://developer.mozilla.org/en-US/docs/Web/MathML/Examples/MathML_Pythagorean_Theorem

Comment 69

[Will Bamberg [:wbamberg]]

jryans, thanks! I've added that stuff: https://developer.mozilla.org/en-US/docs/Tools/View_source.

Comment 70

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

(In reply to Will Bamberg [:wbamberg] from comment #69)
> jryans, thanks! I've added that stuff:
> https://developer.mozilla.org/en-US/docs/Tools/View_source.

Thanks, looks good to me!

Comment 71

[Fernando García Gómez, stripTM]

(In reply to Will Bamberg [:wbamberg] from comment #69)
> jryans, thanks! I've added that stuff:
> https://developer.mozilla.org/en-US/docs/Tools/View_source.

Spanish translation https://developer.mozilla.org/es/docs/Tools/View_source

Comment 73

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

Attachment: MozReview Request: Bug 1067325 - DevTools test fixes for view source in tab. r=pbrosset

Comment 74

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

Attachment: MozReview Request: Bug 1067325 - MathML source in tab. r=mconley

Comment 75

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

Attachment: MozReview Request: Bug 1067325 - Control syntax highlighting via a class. r=hsivonen

Comment 76

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

Attachment: MozReview Request: Bug 1067325 - Fix line endings in several browser / toolkit files. r=mconley

Comment 77

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

Attachment: MozReview Request: Bug 1067325 - Context menu actions for view source tabs. r=mconley

Comment 78

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

Attachment: MozReview Request: Bug 1067325 - Allow DevTools to use view source in tab. r=jsantell

Comment 79

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

Attachment: MozReview Request: Bug 1067325 - Selection source in tab via frame scripts. r=mconley

Comment 80

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

Attachment: MozReview Request: Bug 1067325 - Extract view source core to support tabs. r=mconley

Comment 81

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

Attachment: MozReview Request: Bug 1067325 - Update charset when history is disabled. r=mconley

Comment 82

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

Release note added to Firefox 41.0a2