Plugin click-to-play "don't ask again for this site" checkbox does not work for file URIs

RESOLVED FIXED in Firefox 19

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: Santtu Lakkala, Assigned: Santtu Lakkala)

Tracking

Trunk
Firefox 19
ARM
Android
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121010234852

Steps to reproduce:

I loaded an html file on sdcard (file:///sdcard/index.html) that uses a plugin via the <object> tag. When a pop-up appeared asking whether I wanted to run the plugin, ensured that the "don't show again" checkbox was checked and clicked "Yes"


Actual results:

Pretty much nothing happened. adb logcat shows a js error from nsIPermissionManager.add.


Expected results:

The plugin should be run, and potentially the pop-up should not appear for subsequent tries.
(Assignee)

Comment 1

5 years ago
Created attachment 672243 [details] [diff] [review]
Patch that removes the checkbox for URIs with no host

This is one way of fixing the bug. Simply don't show the checkbox for URIs that do not have a host.

Other way of fixing would be to alter permission manager to use something else as a key, if host is null.

Updated

5 years ago
Attachment #672243 - Flags: review?(margaret.leibovic)

Updated

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Created attachment 672561 [details] [diff] [review]
patch to modify permission manager

(In reply to Santtu Lakkala from comment #1)
> Other way of fixing would be to alter permission manager to use something
> else as a key, if host is null.

I think this is the right way to go.
Attachment #672561 - Flags: review?(mconnor)
Attachment #672561 - Attachment is patch: true
(In reply to David Keeler from comment #2)
> I think this is the right way to go.

See bug 204285. Particularly bug 204285 comment 34, where sicking had some objections to this approach.
So I think the best fix would be to take the front-end fix to just disable the menu item when it doesn't work, and leave the permission manager changes to another bug (bug 204285 I guess?).
Comment on attachment 672561 [details] [diff] [review]
patch to modify permission manager

Ok - we'll have to make the front-end changes in browser-plugins.js as well.
Attachment #672561 - Attachment is obsolete: true
Attachment #672561 - Flags: review?(mconnor)

Comment 7

5 years ago
Comment on attachment 672243 [details] [diff] [review]
Patch that removes the checkbox for URIs with no host

Sorry for the slow review! This looks good to me. Please also add a comment explaining that we need a valid uri.host to set a permission, so that's why we're doing this check.

Note to myself, this won't break the callback functions because aChecked will just be null instead of false:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#5610
Attachment #672243 - Flags: review?(margaret.leibovic) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 675480 [details] [diff] [review]
Updated patch with descriptive comment

Patch with a comment =)

Comment 9

5 years ago
Awesome, thanks!

I was going to check your patch in for you now, but the tree is closed because of build infrastructure issues. Since I can't land the patch right now, you could create a patch that's formatted to be checked in and practice using the checkin-needed keyword. See:
https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in
(Assignee)

Comment 10

5 years ago
Created attachment 675648 [details] [diff] [review]
Updated patch with correct format

Patch in better format.
Attachment #672243 - Attachment is obsolete: true
Attachment #675480 - Attachment is obsolete: true

Comment 11

5 years ago
Thanks! I also added "r=margaret" to the end of the commit message, since all commit messages need to include a reviewer.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d857107b778

This will be merged to mozilla-central soon, and then work its way into Firefox 19.
Assignee: nobody → inz
OS: Linux → Android
Hardware: x86_64 → ARM
Target Milestone: --- → Firefox 19
https://hg.mozilla.org/mozilla-central/rev/1d857107b778
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Now that bug 815640 has landed and file URIs can be used by the permission manager, maybe we should revert this.
You need to log in before you can comment on or make changes to this bug.