849564 - Change - Add a view-source option into the app bar menu

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: App Bar, defect, P2)

Description

[Brian R. Bondy [:bbondy]]

The app bar settings icon currently has only Find in Page. It would be easy and great to also add a view source button into that same menu. 

Source of the idea:
http://www.reddit.com/r/IAmA/comments/19x7up/we_are_the_firefox_user_experience_team_this_is/c8skqa8

Comment 1

[Asa Dotzler [:asa]]

Interesting idea. I think we should look at this for v2 or possibly leave it until we get extension support. (If we only have one item in the settings icon, we should probably float that item up to the top level. What do you think Yuan?)

Comment 2

[Brian R. Bondy [:bbondy]]

Sorry my bad, there's actually 2 items right now and sometimes 1.
There's a view on desktop option for web sites but not chrome pages. I happened to be on the startUI when I was testing it where there is only currently one (find in page).

Comment 3

[Brian R. Bondy [:bbondy]]

I talked to Asa about this and he'd be excited to see us getting a menu item for view source.
Kamil would you be interested in taking this bug?

- It would be a new menu item in the appbar settings icon
- It would not show up on the start page
- When clicked it would open a new tab with something like BrowserUI.addAndShowTab("view-source:" + <current page ur>, Browser.selectedTab);

Comment 4

[Kamil Jozwiak [:kjozwiak]]

Sure, would love to take this one on! Thanks

Comment 5

[Sam Foster [:sfoster] (he/him)]

We'll probably want to disable view-source for view-source: scheme. Looks like we allow view-source for about: pages. Any others to filter out? I guess desktop has a list and/or some logic for this.

Comment 6

[Jim Mathies [:jimm]]

want, but shouldn't block.

Comment 7

[Kamil Jozwiak [:kjozwiak]]

Attachment: added view page source under the app bar settings icon

Went through a few basics tests cases and ensured everything was working:
- Ensure you cannot see the "View page source" item when viewing "view-source:"
- Ensured that you can see the "View page source" entry on all the other websites (http & https)
- Ensured that selecting the "View page source" would open a new tab with the source of the targeted website

Comment 8

[Matt Brubeck (:mbrubeck)]

Comment on attachment 826362 [details] [diff] [review]
added view page source under the app bar settings icon

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

Looks good, thanks!  Some tiny changes, which I can make before landing:

::: browser/metro/base/content/ContextCommands.js
@@ +288,5 @@
> +  viewPageSource: function cc_viewPageSource() {
> +    let uri = this.getPageSource();
> +      if (uri) {
> +        BrowserUI.addAndShowTab(uri);
> +      }

nit: indentation is slightly off here

::: browser/metro/locales/en-US/chrome/browser.dtd
@@ +17,5 @@
> +<!ENTITY appbarJSShell.label         "Open JavaScript shell">
> +<!ENTITY appbarFindInPage2.label     "Find in page">
> +<!ENTITY appbarViewOnDesktop2.label  "View on desktop">
> +<!ENTITY appbarMSMetaData2.label     "Get app for this site">
> +<!ENTITY appbarViewPageSource2.label "View page source">

The '2' isn't necessary here -- we add that if we later need to change the string in a way that makes existing translations invalid.

Comment 9

[Kamil Jozwiak [:kjozwiak]]

Thanks for the review Matt! I can fix the changes tonight when I get home.

Comment 10

[Matt Brubeck (:mbrubeck)]

I went ahead and fixed the nits and landed this, since I was pushing some other patches:
https://tbpl.mozilla.org/?tree=Try&rev=0f37e80c1e62
https://hg.mozilla.org/integration/fx-team/rev/ed1e204e1bcb

Comment 11

[Kamil Jozwiak [:kjozwiak]]

no worries! Thanks Matt, appreciate it.

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/ed1e204e1bcb

Comment 13

[Manuela Muntean [Away]]

Verified as fixed, for iteration #18, with the latest Nightly build (build ID: 20131115030203) on Win 8 64-bit.