Closed Bug 849564 Opened 8 years ago Closed 7 years ago

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

Categories

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

x86_64
Windows 8.1
defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 28

People

(Reporter: bbondy, Assigned: kjozwiak)

References

Details

(Whiteboard: feature=change c=tbd u=tbd p=1)

Attachments

(1 file)

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
Whiteboard: p=2
Flags: needinfo?(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?)
Blocks: metrov2planning
No longer blocks: metrov1triage
Flags: needinfo?(asa) → needinfo?(yuan)
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).
Whiteboard: p=2
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);
Flags: needinfo?(yuan)
Flags: needinfo?(kamiljoz)
Sure, would love to take this one on! Thanks
Flags: needinfo?(kamiljoz)
Assignee: nobody → kamiljoz
Blocks: metrov1backlog
No longer blocks: metrov2planning
Priority: -- → P3
QA Contact: jbecerra
Summary: Add a view-source option into the app bar menu → Change - Add a view-source option into the app bar menu
Whiteboard: feature=change c=tbd u=tbd p=0
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.
want, but shouldn't block.
No longer blocks: metrov1backlog
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
Attachment #826362 - Flags: review?(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.
Attachment #826362 - Flags: review?(mbrubeck) → review+
Thanks for the review Matt! I can fix the changes tonight when I get home.
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
no worries! Thanks Matt, appreciate it.
Blocks: metrov1it18
No longer blocks: metrobacklog
Status: NEW → ASSIGNED
Priority: P3 → P2
Whiteboard: feature=change c=tbd u=tbd p=0 → feature=change c=tbd u=tbd p=1
https://hg.mozilla.org/mozilla-central/rev/ed1e204e1bcb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Verified as fixed, for iteration #18, with the latest Nightly build (build ID: 20131115030203) on Win 8 64-bit.
Status: RESOLVED → VERIFIED
Depends on: 950072
No longer depends on: 950072
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.