Closed Bug 1406903 Opened 2 years ago Closed Last year

cannot view files using content:// scheme

Categories

(Firefox for Android :: General, defect, P5)

All
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 65
Tracking Status
fennec + ---
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed

People

(Reporter: richardp345, Assigned: andrei.a.lazar)

References

Details

Attachments

(3 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170926190823

Steps to reproduce:

Open an .html or .txt file with app EDS Lite 2.x


Actual results:

Filefox not listed as an available app



Expected results:

Firefox should be listed

Issue is that Firefox doesn't have the 'content' scheme as an intent-filter in its manifest.

From the Firefox manifest:

                <data android:scheme="file"/>
                <data android:scheme="http"/>
                <data android:scheme="https"/>

From another browser's manifest:

                <data android:scheme="http"/>
                <data android:scheme="https"/>
                <data android:scheme="about"/>
                <data android:scheme="content"/>
                <data android:scheme="javascript"/>
Also affect Android's in-built file explorer that is available on Marshmallow and up. Sadly it's not as simple adding the missing intent filter, as Firefox then doesn't know how to handle the content:// "protocol".
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → All
Hardware: Unspecified → All
It is sort of possible to convert content:// URIs into file:// URIs to make use of our existing file handling support, but that is rather hacky and not guaranteed to work all the time and across all possible OS versions and content providers, so we should probably immediately forget about that idea again unless we have absolutely no choice.

So the proper way is to use https://developer.android.com/reference/android/content/ContentResolver.html#openInputStream(android.net.Uri), which means a little more work in subsequently getting this into Gecko, though.
In principle we could access the data and store it in a temporary file that could then be accessed via a normal file:// URI, but my preliminary understanding is that if we wanted to preserve the content:// URI on the outside (so history entries/the session store/bookmarks/etc. work correctly and don't refer directly to the temporary file), we'd still have to mess around with the protocol handling somewhat, so we probably might as well go for the full solution, that is implementing a "content" protocol handler that calls out into Java-land to run ContentResolver.openInputStream on the given URI and if successful, then somehow marshals that data back across the JNI border into Gecko to stuff it into an nsIInputStream for further handling by Necko and the rest of Gecko.
tracking-fennec: --- → ?
OS: All → Android
Duplicate of this bug: 1410708
(In reply to Jan Henning [:JanH] from comment #2)
> (so history entries/the session store/bookmarks/etc. work correctly
> and don't refer directly to the temporary file)

Although for that to work we probably have to call https://developer.android.com/reference/android/content/ContentResolver.html#takePersistableUriPermission(android.net.Uri,%20int) and that only works if the app that gave us the content:// URI made it persistable in the first place.
NI Joe and Wesly to help prioritize this
tracking-fennec: ? → +
Flags: needinfo?(wehuang)
Flags: needinfo?(jcheng)
Flags: needinfo?(wehuang)
Priority: -- → P3
Duplicate of this bug: 1444683
(In reply to Jan Henning [:JanH] from comment #4)
> (In reply to Jan Henning [:JanH] from comment #2)
> > (so history entries/the session store/bookmarks/etc. work correctly
> > and don't refer directly to the temporary file)
> 
> Although for that to work we probably have to call
> https://developer.android.com/reference/android/content/ContentResolver.
> html#takePersistableUriPermission(android.net.Uri,%20int) and that only
> works if the app that gave us the content:// URI made it persistable in the
> first place.

Chrome's behaviour is clearly suboptimal - if I open a local file through the inbuilt file manager in Chrome and then kill and restart Chrome, I get an ERR_FILE_NOT_FOUND error when Chrome tries to restore that tab with the content URI.
Unfortunately on the other hand there are some signs that there might be an upper limit on the number of persistable URI permissions an app can take [1], which means that we'd have to attempt releasing unused permissions again. The problem is of course that tracking when a permissions becomes unused, as that URI can be referenced from a load of different places:
- the active URL of a tab
- somewhere in the back/forward history of a tab
- a recently closed tab
- a tab from the previous session
- a bookmark
- browsing history
- ???
and tracking all those places seems rather painful (effectively we'd have to implement garbage collection for persistable URI permissions).


Another problem I've just realised: Opening a local file via a content URI will break HTML files using relative paths (just try it in Chrome and see it yourself). In theory FLAG_GRANT_PREFIX_URI_PERMISSION [2] looks like it would cover that, but
a) the calling app that provides the content URI with its ContentProvider needs to correctly implement that in the first place
b) I'm not sure if you can actually grant an additional file permission for the necessary (although see c) for that) directories at the same time as passing the content URI to the actually intended target file
c) relative paths could go all over the place both down and *up* in the directory structure, so e.g. a file explorer app would have to grant permissions for potentially the whole storage card in case some relative path in that HTML file is referencing it

So from that point of view, sadly it seems like implementing something like [3] and only falling back to the theoretically proper way of using ContentResolver.openInputStream() if we cannot retrieve a working file URI would be the way to go :-(

[1] https://stackoverflow.com/questions/30802971/android-max-number-of-persistable-uri-permission-granted-to-an-app-is-limited-to
[2] https://developer.android.com/reference/android/content/Intent.html#FLAG_GRANT_PREFIX_URI_PERMISSION
[3] https://github.com/iPaulPro/aFileChooser/blob/master/aFileChooser/src/com/ipaulpro/afilechooser/utils/FileUtils.java, if necessary extending it for whatever content URI schemes other popular file managers might be using [4]
[4] Assuming/hoping/asking them nicely that their content URI scheme/Intent allows reverse engineering the file path in some way or other
(In reply to Jan Henning [:JanH] from comment #2)
> Another problem I've just realised: Opening a local file via a content URI
> will break HTML files using relative paths (just try it in Chrome and see it
> yourself).

Out of curiosity I filed https://bugs.chromium.org/p/chromium/issues/detail?id=827801 for that. Let's see what - if anything - they'd do.

(In reply to Jan Henning [:JanH] from comment #7)
> So from that point of view, sadly it seems like implementing something like
> [3] and only falling back to the theoretically proper way of using
> ContentResolver.openInputStream() if we cannot retrieve a working file URI
> would be the way to go :-(

So if in the end we still have to preferentially attempt accessing the original file URI (where possible) to preserve handling of relative URIs, and attempting to persist content URIs has its own problems because not every app might allow that and there might be an upper limit on the number of URIs we can persist, we might as well give up and just

(In reply to Jan Henning [:JanH] from comment #2)
> access the data and store it in a temporary file that
> could then be accessed via a normal file:// URI

In that case we could implement this via the ContentDispatchChooser [1], which we already use to forward unknown protocols first to our Android front end and then to the OS. We could intercept content://-URIs, either retrieve the original file://-URI or create a temporary file ourselves and then return that fixed-up URI to Gecko.

[1] https://dxr.mozilla.org/mozilla-central/rev/0405f6006f3a3f653dd42d587c3eefe08cffa37d/mobile/android/components/ContentDispatchChooser.js
See Also: → 1450449
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: P3 → P5
(In reply to Jan Henning [:JanH] from comment #8)
> (In reply to Jan Henning [:JanH] from comment #2)
> > Another problem I've just realised: Opening a local file via a content URI
> > will break HTML files using relative paths (just try it in Chrome and see it
> > yourself).
> 
> Out of curiosity I filed
> https://bugs.chromium.org/p/chromium/issues/detail?id=827801 for that. Let's
> see what - if anything - they'd do.

And https://issuetracker.google.com/issues/77406791, although I fully expect nothing to happen for two or three years before it'll simply be closed as "Won't Fix (Obsolete)", even though nothing has changed about the situation.
Duplicate of this bug: 1375035
Assignee: nobody → andrei.a.lazar
Added support for opening HTML files from internal storage when user is choosing fennec in the dialog picker.
Comment on attachment 9012854 [details]
Bug 1406903 Part 1: Import code for dealing with content:// URIs from aFileChooser. r=JanH

Vlad Baicu has approved the revision.
Attachment #9012854 - Flags: review+
Added support for opening HTML files from internal storage when user is choosing fennec in the dialog picker.
Attachment #9013713 - Attachment is obsolete: true
Attachment #9013714 - Attachment is obsolete: true
Attachment #9013713 - Attachment is obsolete: false
Attachment #9013714 - Attachment is obsolete: false
Attachment #9013713 - Attachment is obsolete: true
Blocks: 1496079
Attachment #9013714 - Attachment description: Bug 1406903 part 2 r=JanH → Bug 1406903 Part 2: Added support for opening HTML files. r=JanH
Attachment #9012854 - Attachment description: Bug 1406903 cannot view files using content:// scheme r=JanH → Bug 1406903 Part 1: Import code for dealing with content:// URIs from aFileChooser. r=JanH
Keywords: checkin-needed
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05d8e65ff651
Part 1: Import code for dealing with content:// URIs from aFileChooser. r=JanH,VladBaicu
https://hg.mozilla.org/integration/autoland/rev/8a510f2052d3
Part 2: Added support for opening HTML files. r=JanH
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/05d8e65ff651
https://hg.mozilla.org/mozilla-central/rev/8a510f2052d3
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Keywords: regression
Version: Firefox 56 → Firefox 63
This wasn't a regression, we just never got round to implementing this until now.
Flags: needinfo?(jcheng)
Keywords: regression
Version: Firefox 63 → unspecified
Depends on: 1499618
Depends on: 1502721
Backed out from beta as requested by Marcia for causing Bug 1502721:

https://hg.mozilla.org/releases/mozilla-beta/rev/194d3585f3d567c047245c842ac5ee6b6390b474
Flags: needinfo?(andrei.a.lazar)
Target Milestone: Firefox 64 → Firefox 65
This is safe to land now, all defects caused by this feature have been fixed (see Bug 1502721, Bug 1499618 and Bug 1501688). Ryan, should I make an uplift request for this?
Flags: needinfo?(andrei.a.lazar) → needinfo?(ryanvm)
Keywords: checkin-needed
There's nothing to uplift here AFAICT (it was backed out from 64 and 65 is already on Beta). Please do request uplift on bug 1501688 after it gets merged to m-c, though.
Flags: needinfo?(ryanvm)
Removing the checkin-needed until there's something to be uplifted here. Andrei feel free to put it back if you have something to be landed on integration branches or on beta. Thanks
Keywords: checkin-needed
See Also: → 1516467
Depends on: 1501688
Attachment #9012854 - Attachment is obsolete: true
Attachment #9013714 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.