SDK-based addons prevent FF from loading links clicked from an app on start

VERIFIED FIXED in 1.4

Status

P1
normal
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: canuckistani, Assigned: mossop)

Tracking

unspecified
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reported in IRC by 'honestbleeps', based on this screencast:

http://www.youtube.com/watch?v=oJ-TWhogJkE&feature=youtu.be

To reproduce:

0. ensure Firefox is the default browser, eg it will be opened when you click on a link from another app ( Twitter, etc )
1. disable all addons, then shut down Firefox
2. click on a link in the Twitter client

Result: Firefox opens and switches to a new tab that displays the link you clicked on.

3. next, enable a single, SDK-based addon such as this one: 
http://dl.dropbox.com/u/44296964/empty.xpi
4. shut down Firefox, then click on a link in the twitter client

Result: Firefox starts up but does *not* open a new tab that displays the link you clicked on.
Summary: some addons prevent FF from loading links clicked from an app → SDK-based addons prevent FF from loading links clicked from an app
Summary: SDK-based addons prevent FF from loading links clicked from an app → SDK-based addons prevent FF from loading links clicked from an app on start
Seems this is a regression in 1.3, as Jeff was unable to reproduce with 1.3.

The new loader, probably?
(Assignee)

Comment 2

7 years ago
Matteo, can you dive into this? We want to see if we can get a fix for this before the 1.4 release (Tuesday). We suspect the new loader but I guess a bisect can confirm that. Irakli will be back on Monday if needed to help out but we'd like you to get started on this right away.
Assignee: nobody → zer0
Blocks: 714820
Priority: -- → P1
Target Milestone: --- → 1.4

Comment 3

7 years ago
Hey! I'm the creator of the screencast and such. If you need any other bits of information (logs/dumps/etc), please let me know! I don't know if there's anything else you need, but I figured it'd be polite to at least poke my nose in and make the offer. 

P.S. - You guys do excellent work. Thank you!
Dave: isn't Matteo on PTO?
So I did a git bisect on this between 1.3 and 1.4rc3.

>	0517e96d425060402ac9e7c519dedfeead49a932 is the first bad commit
	commit 0517e96d425060402ac9e7c519dedfeead49a932
	Author: Irakli Gozalishvili <rfobic@gmail.com>
	Date: Tue Aug 9 11:55:54 2011 +0200
	Actually replacing `bootstrap.js` with `loader.js`.
	:040000 040000 16fd7efdc1106500ad11d15bc254647566c7fd04 a33ec3e2e1fda39de4f4abdc51bb4a8258f6b404 M packages
	:040000 040000 59ccc076460cdce0369cf739bf2eaf0a36b0d70a 232c38e0768ee0461260492e6f6f3b8525df2ab6 M python-lib
Created attachment 586468 [details] [diff] [review]
Do an async XMLHttpRequest

So Jeff figured out that this bug is caused by the following line in bootstrap.js
  let options = JSON.parse(readURI(URI + './harness-options.json'));

I'd imagine that this is due to the synchronous XMLHttpRequest.
Here is a patch that allows to use an asynchronous one.
(Assignee)

Comment 7

7 years ago
Created attachment 586476 [details] [diff] [review]
Use synchronous nsIChannel

This is an alternate patch that I think is probably safer as it avoids race conditions and keeps things basically as they were. Sync XHR does all kinds of nasty things with event loops and script blocking as I recall which is probably where our issue is coming from, this on the other hand does pure synchronous I/O to get the data.
Do we know what was actually causing Firefox to fail to load the URI? Seems like that shouldn't be possible from this problem. Should a followup bug be filed for an internal fix, better error handling or whatnot?
(Assignee)

Comment 9

7 years ago
(In reply to Dietrich Ayala (:dietrich) from comment #8)
> Do we know what was actually causing Firefox to fail to load the URI? Seems
> like that shouldn't be possible from this problem. Should a followup bug be
> filed for an internal fix, better error handling or whatnot?

I'm interested in this too so I'm going to play around in a debug build later today and see what turns up.

Comment 10

7 years ago
Commits pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/543cf350352b62428e08a6d864517fbadcaa3d5f
Bug 715721: Switch to using nsIChannel to do the synchronous I/O load from a URI. r=@mykmelez

https://github.com/mozilla/addon-sdk/commit/b66482ba0facfa8585fb10ae172e176af4146e21
Merge pull request #321 from Mossop/bug715721

fix Bug 715721: Switch to using nsIChannel to do the synchronous I/O load from a URI. r=@mykmelez

Comment 11

7 years ago
Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fedf331d99dfd890cb33e8606fd5ed23425ef750
fix bug 715721: Switch to using nsIChannel to do the synchronous I/O load from a URI. r=@mykmelez
(cherry picked from commit 543cf350352b62428e08a6d864517fbadcaa3d5f)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Assignee: zer0 → dtownsend
(Assignee)

Comment 12

7 years ago
(In reply to Dave Townsend (:Mossop) from comment #9)
> (In reply to Dietrich Ayala (:dietrich) from comment #8)
> > Do we know what was actually causing Firefox to fail to load the URI? Seems
> > like that shouldn't be possible from this problem. Should a followup bug be
> > filed for an internal fix, better error handling or whatnot?
> 
> I'm interested in this too so I'm going to play around in a debug build
> later today and see what turns up.

Ok I believe I know the issue here and it really is our fault, this particular symptom is the most obvious but there may have been other problems too.

On Linux and Windows new URLs are opened by passing them on the command line so everything is fine there, when we get to the appropriate point in startup we parse the command line and load the urls. OSX does things quite differently. Instead it sends an OS event to the application to tell it to load the URL. In Gecko we register a thing called an application delegate. When we process events in the OS event queue some of them are automatically passed along to the app delegate to handle loading URLs and files.

The problem is that a sync XHR causes us to spin an event loop early in startup, before the delegate is registered (also likely before anything would be ready to handle opening a URL anyway). This event loop sees and effectively discards the waiting event instructing us to load a URL.
I've confirmed that the current stabilization branch ( @ fedf331 ) no longer suffers from this issue. Additionally, all tests pass.
Status: RESOLVED → VERIFIED

Comment 14

7 years ago
Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fedf331d99dfd890cb33e8606fd5ed23425ef750
fix bug 715721: Switch to using nsIChannel to do the synchronous I/O load from a URI. r=@mykmelez
You need to log in before you can comment on or make changes to this bug.