Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@GM_getResourceURL won't work for setting favicon #1955

Closed
vBm opened this issue Jul 12, 2014 · 18 comments
Closed

@GM_getResourceURL won't work for setting favicon #1955

vBm opened this issue Jul 12, 2014 · 18 comments
Milestone

Comments

@vBm
Copy link

vBm commented Jul 12, 2014

I'm trying to replace existing favicon of a website with a custom one (so i can indicate that my script 'tweaked' the page)

This is simple testcase. -> https://gist.github.com/vBm/85a254006b6182bb59ba

While this code works perfectly fine using chrome with tampermonkey, on firefox and GM it's not working.

Hope this is 'good' enough report to get it fixed. If you need more info, i'll be eager to provide it ASAP.
Thanks in advance.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0
GM 2.0

@arantius
Copy link
Collaborator

The coment on the gist suggests it is this specific usage that is the problem? Can you confirm?

@vBm
Copy link
Author

vBm commented Jul 14, 2014

No, even with changed code i still couldn't make it work on firefox/gm while it works just fine in chrome/tm :x
Can anyone else confirm the issue ?

@janekptacijarabaci
Copy link
Contributor

Yes, I confirm this. It really does not work correctly. But it looks like a Firefox problem.

I have the same problem with:
http://mozorg.cdn.mozilla.net/media/img/favicon.ico
or
https://mozorg.cdn.mozilla.net/media/img/favicon.ico

Firefox 30.0 - 33.0a1 (2014-07-15, Nightly)

1

@arantius arantius changed the title @GM_getResourceURL isn't working @GM_getResourceURL won't work for setting favicon Jul 16, 2014
@arantius
Copy link
Collaborator

Edited the subject.

https://gist.github.com/arantius/195e4226fd7a8500cf4e

GM_getResourceURL definitely works, as per above test case. It just doesn't work (in that particular way?) to alter the current tab's favicon. Something about how the browser makes the request is incompatible with our custom scheme handler.

@arantius arantius added this to the Pony milestone Jul 16, 2014
@vBm
Copy link
Author

vBm commented Jul 16, 2014

It was mistake on my part for not explicitly saying it.
Thanks for confirming.
Please note that this used to work, but i can't remember when was the last time i've noticed it.

Ventero added a commit to Ventero/greasemonkey that referenced this issue Sep 7, 2014
This ensures URIs containing reserved characters for URI components
(like "#") are handled correctly. Previously, any modification to
such an URI's hash would lead to mis-parsing of the resource name.

One example where we don't fully control the URI's hash are favicons,
which have their icon size appended to the hash.

Fixes greasemonkey#1955.
@Ventero
Copy link
Contributor

Ventero commented Sep 7, 2014

Ventero@ca8de27 fixes this issue.

As it turns out, Firefox doesn't always use the exact link.href as the favicon URI for its request. Instead, it may add size information to the URI's hash (see tabbrowser#setIcon and PlacesUtils.getImageURLForResolution), which for greasemonkey-script URIs we would then interpret as part of the resource name. Thus, we would end up looking for a resource with a name like foo#-moz-resolution=16,16.

To fix this issue, we simply have to ignore URI's hash when parsing the resource name out of the URI. However, this would break resources with reservered characters in their name - so we have to use the encoded resource name in the URI's path instead, thus ensuring that any reserved characters are properly escaped.

Strictly speaking, this is a backwards incompatible change that affects any script that uses a stored/pre-generated resource URI containing characters that are reserved in URI components (if there are even any scripts that do this). However, I'd argue that this isn't a big issue, as the documentation for @resource specifies that the resource name should comply with JS identifier restrictions, which rules out any of the abovementioned reserved characters.

(Edit: Pushed a new commit to fix a typo.)

Ventero added a commit to Ventero/greasemonkey that referenced this issue Sep 7, 2014
This ensures URIs containing reserved characters for URI components
(like "#") are handled correctly. Previously, any modification to
such an URI's hash would lead to mis-parsing of the resource name.

One example where we don't fully control the URI's hash are favicons,
which have their icon size appended to the hash.

Fixes greasemonkey#1955.
@arantius arantius modified the milestones: 2.3, Pony Sep 7, 2014
@arantius arantius modified the milestones: 2.3, 2.4 Sep 21, 2014
@arantius arantius modified the milestones: 2.4, 2.5 Oct 29, 2014
@arantius arantius modified the milestones: 3.3, 3.1 Mar 16, 2015
@arantius
Copy link
Collaborator

When I install the test case script linked in the original description today, it appears to work. I see the favicon from the script on my tab.

Can anyone confirm/refute?

@janekptacijarabaci
Copy link
Contributor

I confirmed. But...
Firefox 38.0.1, Firefox Nightly 41.0a1 (2015-05-29) only [e10s off]:
error

@arantius
Copy link
Collaborator

Curious, I don't get that with e10s on or off, in 41.0a1.

@janekptacijarabaci
Copy link
Contributor

Yes :-)

I don't know why, but not confirmed on:
Ubuntu 15.04 64-bit
Firefox 38.0, Firefox Nightly 41.0a1 (2015-05-29) [e10s on/off]

I confirmed on:
Windows 7 and 8.1 64-bit (even in a clean profile)
Firefox 38.0.1, Firefox Nightly 41.0a1 (2015-05-29) only [e10s off]

Can anyone confirm, please?

@arantius
Copy link
Collaborator

Same machine, 38.0, no error (e10s off, not available yet in 38).

@vBm
Copy link
Author

vBm commented May 29, 2015

I can confirm that it works just fine now on (as intended)

GM 3.2 + Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0

@janekptacijarabaci
Copy link
Contributor

@vBm: Do you have any error in the console?

@vBm
Copy link
Author

vBm commented May 29, 2015

Do you have any error in the console?

None.

@janekptacijarabaci
Copy link
Contributor

It's strange. But thanks.

@arantius
Copy link
Collaborator

On Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0 I get

NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsISyncMessageSender.sendSyncMessage] scriptProtocol.js:118:0
[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsISyncMessageSender.sendSyncMessage]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: resource://greasemonkey/scriptProtocol.js :: ScriptProtocol.newChannel :: line 118"  data: no] tabbrowser.xml:547:0

But it continues to work. Oh I bet I know why. The code just did

    var mm = Cc["@mozilla.org/childprocessmessagemanager;1"]
        .getService(Ci.nsISyncMessageSender);

But this is the parent process, getting the image to go in the UI chrome. Or at least, why the error. Still unsure how the image then shows up in the tab though.

@arantius
Copy link
Collaborator

Can I say again how much I hate e10s? It's so damn hard to work with. "Oh, J/K LOL that kind of message manager isn't the one I wanted this time! Sucks to be you!"

@arantius
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants