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

VERIFIED FIXED in Firefox 28

Status

defect
P2
normal
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: bbondy, Assigned: kjozwiak)

Tracking

Trunk
Firefox 28
x86_64
Windows 8.1

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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
(Reporter)

Updated

6 years ago
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)
(Reporter)

Comment 2

6 years ago
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
(Reporter)

Comment 3

6 years ago
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)
(Assignee)

Comment 4

6 years ago
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
(Assignee)

Comment 7

6 years ago
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+
(Assignee)

Comment 9

6 years ago
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
(Assignee)

Comment 11

6 years ago
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
Last Resolved: 6 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.