Closed
Bug 1169091
Opened 9 years ago
Closed 9 years ago
Show error page when opening URI with custom scheme and no application to handle available
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1162372
People
(Reporter: mcomella, Unassigned, Mentored)
References
Details
(Whiteboard: [lang=js])
Attachments
(1 file)
108.11 KB,
image/png
|
Details |
If you try to open a link with a custom url scheme (e.g. [1]), the associated app should open if applicable. However, if the app is not installed, we currently display a toast and with a button to request to search the Play Store. Wes thought we should replace this toast with an error page that has a link to search the Play Store.
Implmentation likely starts with removing the code at [2] and showing the error page instead.
We also show this toast for intent:// URIs and we were looking to just outright search the store when nothing was installed (bug 1168980).
[1]: https://people.mozilla.org/~wjohnston/sonos.html
[2]: http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/ContentDispatchChooser.js#52
Comment 1•9 years ago
|
||
I've started working on this, any chance it could be assigned to me?
I don't have any real code to show yet - the new redirect works, and I'm working on making the new error page functional (i.e. actually searching the appstore). Currently the message is all part of a new about: page, but I'm thinking of moving it into the about:neterror page instead (since that already handles unknown protocols).
I've attached a screenshot of what things look like at the moment (please ignore the colour, I started by modifying the about:certerror page, whereas plain grey would make more sense for this).
The things I'm unsure about:
- What's the most sensible wording here?
- Should the loaded url be hidden by default (we could have an expandable section similar to the "technical details" in about:certerror)?
(-If it isn't merged into about:neterror, would about:schemenotsupported be a good name?)
Comment 2•9 years ago
|
||
Ooops, I got completely confused here, it seems this issue is already fixed by [1]? (I first tested on release FF (39.0 for me) to see the toast that appears when a scheme isn't supported, and then started hacking on up to date mozilla-central without actually testing what happens there first...)
(And based on the comments there, it seems the idea of linking to the play store is dead, so probably no point in me trying to work on that.)
[1] https://hg.mozilla.org/mozilla-central/rev/d43113d59aab
Comment 3•9 years ago
|
||
But, another thing I was looking at was loading the error page without having to reveal the about:neterror?e=...&u=foo:bar url in the address bar (i.e. keeping "foo:bar" in the address bar if it's unloadable, while still showing the about:neterror page).
For certificate and loading errors, the url of the original page being loaded is still displayed in the address bar (as opposed to displaying about:certerror/neterror, which is what is loaded in the background), whereas here the (non-openable) url is overwritten with the about:neterror url - it would probably be nicer to keep the desired url showing in the url bar? (I.e. if we try to load, but can't open foo:bar, we still have "foo:bar" instead of about:neterror?e=....&u=foo:bar in the address bar.)
It seems to be quite tricky to actually do that though - right now the only place that we load about:certerror/neterror in this way is in nsDocShell, using LOAD_FLAGS_ERROR_PAGE to avoid overwriting the desired page url. This flag is [1] an internal only define, and only parsed in nsDocShell::InternalLoad [2] which is native only - it seems like we're only able to access nsDocShell::LoadURI from js code, which only passes a limited selection of flags on to InternalLoad [3].
It doesn't seem like it would be difficult to expose LOAD_FLAGS_ERROR_PAGE to frontend js - but I'm guessing that's not a good idea security-wise? (I'm quite new to the codebase though, so don't have much of a clue in this area, and might also have misunderstood the other bits of code I've read.)
[1] http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShellLoadTypes.h#19
[2] http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10379
[3] http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#1635
Updated•9 years ago
|
Flags: needinfo?(michael.l.comella)
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to Andrzej Hunt :ahunt from comment #2)
> Ooops, I got completely confused here, it seems this issue is already fixed
> by [1]?
Yeah, sorry about that – this was fixed by bug 1162372.
> (And based on the comments there, it seems the idea of linking to the play
> store is dead, so probably no point in me trying to work on that.)
We actually do link to the play store in the event that an explicit package is provided in the Intent (so it's not so much a search as a listing), which is what Chrome does. Bug 1192436 is also open to improve this behavior.
(In reply to Andrzej Hunt :ahunt from comment #3)
> it would probably be nicer to keep the desired url showing in the url
> bar? (I.e. if we try to load, but can't open foo:bar, we still have
> "foo:bar" instead of about:neterror?e=....&u=foo:bar in the address bar.)
I agree! We didn't do this for time reasons more than anything else – I filed bug 1193907. Let's continue this conversation there.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(michael.l.comella)
Resolution: --- → DUPLICATE
Assignee | ||
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
•