Closed
Bug 771915
Opened 12 years ago
Closed 12 years ago
Show the origin of the page in the chrome of the app when it is different from webapp's origin
Categories
(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)
Tracking
(firefox17 fixed, firefox18 verified)
VERIFIED
FIXED
Firefox 18
People
(Reporter: jsmith, Assigned: wesj)
References
Details
(Keywords: uiwanted, Whiteboard: [blocking-webrtandroid1+])
Attachments
(3 files, 7 obsolete files)
207.74 KB,
image/png
|
Details | |
10.41 KB,
patch
|
mfinkle
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
399.92 KB,
image/png
|
Details |
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
It's all possible to do, but this needs UX input to figure out how to do it.
Reporter | ||
Updated•12 years ago
|
Blocks: Blocking-FFA-WebRT1+
Priority: -- → P1
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → wjohnston
Assignee | ||
Comment 3•12 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•12 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•12 years ago
|
||
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•12 years ago
|
Whiteboard: [blocking-webrtandroid1+]
Assignee | ||
Comment 6•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
Attachment #642152 -
Attachment is obsolete: true
Attachment #645603 -
Attachment is obsolete: true
Attachment #642152 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 9•12 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•12 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 11•12 years ago
|
||
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•12 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•12 years ago
|
||
Btw, looking at the screenshot, are you showing the full URL or just the origin?
Reporter | ||
Comment 15•12 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•12 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.
Comment 17•12 years ago
|
||
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.
Comment 18•12 years ago
|
||
(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.
Comment 19•12 years ago
|
||
(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.
Comment 20•12 years ago
|
||
(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
Comment 21•12 years ago
|
||
(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•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
Grrr. forgot to qref.
Attachment #651870 -
Attachment is obsolete: true
Attachment #651870 -
Flags: review?(mark.finkle)
Attachment #651872 -
Flags: review?(mark.finkle)
Comment 25•12 years ago
|
||
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•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 26•12 years ago
|
||
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•12 years ago
|
||
Comment 29•12 years ago
|
||
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•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8264b7d01a5a
Comment 31•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8264b7d01a5a
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Reporter | ||
Updated•12 years ago
|
status-firefox17:
--- → affected
Assignee | ||
Comment 32•12 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•12 years ago
|
Status: RESOLVED → VERIFIED
status-firefox18:
--- → verified
Comment 33•12 years ago
|
||
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+
Updated•12 years ago
|
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•