The default bug view has changed. See this FAQ.

Show the origin of the page in the chrome of the app when it is different from webapp's origin

VERIFIED FIXED in Firefox 17

Status

()

Firefox for Android
Web Apps
P1
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: jsmith, Assigned: wesj)

Tracking

({uiwanted})

Trunk
Firefox 18
ARM
Android
uiwanted
Points:
---

Firefox Tracking Flags

(firefox17 fixed, firefox18 verified)

Details

(Whiteboard: [blocking-webrtandroid1+])

Attachments

(3 attachments, 7 obsolete attachments)

(Reporter)

Description

5 years ago
Flagged by security for desktop in bug 771395, so likely applies to Android as well. If an app goes off the origin of the app origin, it needs to show the current origin of the page in the chrome of the web app. An example use case:

1. Launch mozilla marketplace <-- nothing in the chrome yet
2. Click login <-- pop-up would have chrome with BID as the origin
(Reporter)

Updated

5 years ago
See Also: → bug 771395
Vlad/Wes, thoughts?
It's all possible to do, but this needs UX input to figure out how to do it.
(Reporter)

Updated

5 years ago
Keywords: uiwanted
(Reporter)

Updated

5 years ago
Blocks: 766259
Priority: -- → P1
(Reporter)

Updated

5 years ago
Assignee: nobody → wjohnston
(Assignee)

Comment 3

5 years ago
Idea? Show the Android titlebar in these cases, and use setTitle() (which will work here) to make it show the origin?
Priority: P1 → --
(Reporter)

Comment 4

5 years ago
(In reply to Wesley Johnston (:wesj) from comment #3)
> Idea? Show the Android titlebar in these cases, and use setTitle() (which
> will work here) to make it show the origin?

Works for me.
Priority: -- → P1
(Assignee)

Comment 5

5 years ago
Created attachment 642152 [details] [diff] [review]
Patch

This shows a simple titlebar at the top of the screen when you're on a third party and hides it when you're on the trusted app's origin. Tested with Facebook and Twitter links on Jauntly.

Its a bit ugly right now, but I can live with that.
Attachment #642152 - Flags: review?(mark.finkle)
(Reporter)

Updated

5 years ago
Whiteboard: [blocking-webrtandroid1+]
(Assignee)

Comment 6

5 years ago
Created attachment 645603 [details]
Screenshot

This is a screenshot of this (with the url moved to the bottom of the screen). Probably needs a shadow or a line dividing it from content and a little padding. Also, its shifting content up (i.e. we need to relayout).
(Assignee)

Comment 7

5 years ago
Created attachment 648112 [details] [diff] [review]
Patch

This seems to work well. Very unobtrusive. Some amazing art by me. Shows the url at the bottom of the screen. Sadly, its spoofable if the app wants to pretend to be off-origin when its not. I'll post a screenshot.
Attachment #648112 - Flags: review?(mark.finkle)
(Assignee)

Comment 8

5 years ago
Created attachment 648114 [details]
Screenshot
Attachment #642152 - Attachment is obsolete: true
Attachment #645603 - Attachment is obsolete: true
Attachment #642152 - Flags: review?(mark.finkle)
(Assignee)

Comment 9

5 years ago
That's a bit hard to read. Looks better on my device, but its still light colored. Easy way to test is to use ed.agadak.net/app, install the ed agadak app which has links to google/youtube/etc.
(Reporter)

Comment 10

5 years ago
(In reply to Wesley Johnston (:wesj) from comment #7)
> Created attachment 648112 [details] [diff] [review]
> Patch
> 
> This seems to work well. Very unobtrusive. Some amazing art by me. Shows the
> url at the bottom of the screen. Sadly, its spoofable if the app wants to
> pretend to be off-origin when its not. I'll post a screenshot.

Can you clarify how an app could do this? Could this be a security concern?
Comment on attachment 648112 [details] [diff] [review]
Patch

Overall this approach is fine, but I want Ian to take a look at the screenshot and offer some tweaks.

For example:
* Top or bottom of the screen? Bottom is more like a statusbar, but Android has no such concept.
* Should we mimic the normal Android title bar a bit more?
Attachment #648112 - Flags: review?(mark.finkle) → feedback+
(Assignee)

Comment 12

5 years ago
Sounds good. I can put up a build as well as some instructions too. I found the appearance of the titlebar at the top distracting. It would appear and shove content down. Maybe a nice transition would help (but that would be a layout transition which Android kinda hates). I'd rather do that later though.
(Reporter)

Comment 13

5 years ago
Btw, looking at the screenshot, are you showing the full URL or just the origin?
(Assignee)

Comment 14

5 years ago
full url
(Reporter)

Comment 15

5 years ago
(In reply to Wesley Johnston (:wesj) from comment #14)
> full url

Oh. For meeting the original security requirement, I believe you only need to show the origin (developer.mozilla.org if you were on developer.mozilla.org/en-US/Apps), not the full URL. Not sure of the tradeoffs of only showing the origin vs. the full URL.
(Assignee)

Comment 16

5 years ago
I talked to ian on irc. He said he'd rather the url was at the top. I'll make that adjustment and file a follow up to animate it? He said he can do a mockup, but I'm fine moving that somewhere else for follow up prettifying.
Created attachment 650891 [details]
Mockup

Attached is a mockup describing where we'd like to take the design of this bar. Basically, it should look like a micro version of the title bar. 

I have broken the design into two parts here. The first part takes what Wes has already done and adds some styling. The second part adds a few elements that might also be useful down the road, like a place for the Globe / Lock icons, and potentially a progress indicator while the page is loading. 

One possible approach might be to do the first part now for 16, and the second for 17.
(In reply to Ian Barlow (:ibarlow) from comment #17)
> Basically, it should look like a micro version of the title bar. 
> 

Sorry, I meant to say a micro version of the *Firefox* title bar there.
(In reply to Ian Barlow (:ibarlow) from comment #17)
> I have broken the design into two parts here. The first part takes what Wes
> has already done and adds some styling. The second part adds a few elements
> that might also be useful down the road, like a place for the Globe / Lock
> icons, and potentially a progress indicator while the page is loading. 

Over in bug 772680, we decided not to show a throbber when an app is doing network activity.  The rationale is not described in the bug, but the summary is that doing so should be the responsibility of the application, not the runtime, because such runtime behavior is conventional (cf. the Java and Adobe Air runtimes), and because the semantics of network activity are complex in modern web apps, and the runtime does not have enough context to present progress indicators on their apps' behalves.

The latter is especially true when the indication is of the loading of a page, since modern webapps are often constructed with lightweight pages that load content dynamically after the static page and its statically-declared resources have loaded.

So I don't think we should be showing a progress indicator for these pages.  Nor show other UI that is designed to emulate browsing affordances.  The goal here is for the normally-invisible runtime to intrude on the user's experience of apps to the minimum extent necessary to protect users from the consequences of malicious or insecure apps.
(In reply to Myk Melez [:myk] [@mykmelez] from comment #19)

> So I don't think we should be showing a progress indicator for these pages. 
> Nor show other UI that is designed to emulate browsing affordances.  The
> goal here is for the normally-invisible runtime to intrude on the user's
> experience of apps to the minimum extent necessary to protect users from the
> consequences of malicious or insecure apps.

That's OK. We can move forward on the basic styling and perhaps the globe
(In reply to Mark Finkle (:mfinkle) from comment #20)
> (In reply to Myk Melez [:myk] [@mykmelez] from comment #19)
> 
> > So I don't think we should be showing a progress indicator for these pages. 
> > Nor show other UI that is designed to emulate browsing affordances.  The
> > goal here is for the normally-invisible runtime to intrude on the user's
> > experience of apps to the minimum extent necessary to protect users from the
> > consequences of malicious or insecure apps.
> 
> That's OK. We can move forward on the basic styling and perhaps the globe

Yup, sounds good.
(Assignee)

Comment 22

5 years ago
Created attachment 651861 [details] [diff] [review]
Patch

This move this to the top and styles it using an Android gradient.
Attachment #648112 - Attachment is obsolete: true
Attachment #648114 - Attachment is obsolete: true
(Assignee)

Comment 23

5 years ago
Created attachment 651870 [details] [diff] [review]
Patch

Patch v2 - This changes this to just show the protocol + host
Attachment #651861 - Attachment is obsolete: true
Attachment #651870 - Flags: review?(mark.finkle)
(Assignee)

Comment 24

5 years ago
Created attachment 651872 [details] [diff] [review]
Patch v2

Grrr. forgot to qref.
Attachment #651870 - Attachment is obsolete: true
Attachment #651870 - Flags: review?(mark.finkle)
Attachment #651872 - Flags: review?(mark.finkle)
Comment on attachment 651872 [details] [diff] [review]
Patch v2

* Is the webapp_titlebar_bg.xml Ian-approved? :)
* You add webapp_titlebar_bg.xml into the Makefile twice, but only have one actual file (I think). Is that right?
* Do we need to import AnimationUtils? I don't see it explicitly used.
* SharedPreferences prefs does not seem to be used
* Getting the "app" using the index created from a string seems weak. Do we have any other ways to look it up?
Attachment #651872 - Flags: review?(mark.finkle) → review-

Updated

5 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 26

5 years ago
Created attachment 654718 [details] [diff] [review]
Patch v3

Updated patch. I added a "getIndex" function that's defined for WebApp# in the fragment. Good idea. I'll attach a screenshot too and ping ibarlow for approval.
Attachment #651872 - Attachment is obsolete: true
Attachment #654718 - Flags: review?(mark.finkle)
(Assignee)

Comment 27

5 years ago
Created attachment 654719 [details]
Screenshot
(Assignee)

Comment 28

5 years ago
irc approval from ian with a change in the text color to 666666
Comment on attachment 654718 [details] [diff] [review]
Patch v3

>diff --git a/mobile/android/base/WebAppsFragment.java.frag b/mobile/android/base/WebAppsFragment.java.frag

>+    protected int getIndex() { return @APPNUM@; }

Nice

>diff --git a/mobile/android/base/resources/drawable/webapp_titlebar_bg.xml b/mobile/android/base/resources/drawable/webapp_titlebar_bg.xml

>\ No newline at end of file

nit: add a newline
Attachment #654718 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 30

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8264b7d01a5a
https://hg.mozilla.org/mozilla-central/rev/8264b7d01a5a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
(Reporter)

Updated

5 years ago
status-firefox17: --- → affected
(Assignee)

Comment 32

5 years ago
Comment on attachment 654718 [details] [diff] [review]
Patch v3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Webapps
User impact if declined: Security concerns with sites that take people to an unexpected off-origin url
Testing completed (on m-c etc.): Landed on mc 8/27
Risk to taking this patch (and alternatives if risky): Low risk. Webapps only
String or UUID changes made by this patch: None.
Attachment #654718 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Status: RESOLVED → VERIFIED
status-firefox18: --- → verified
Comment on attachment 654718 [details] [diff] [review]
Patch v3

Low risk, webapps only (targeted for 17) approving.
Attachment #654718 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 34

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/e3a804ecaa08

Updated

5 years ago
status-firefox17: affected → fixed
You need to log in before you can comment on or make changes to this bug.