Don Kleppinger Reporter
	Description • 9 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0 cadebug
Build ID: 20150525141253

Steps to reproduce:

We have a customized branded version of firefox for an internal enterprise web app.
I'm trying to upgrade and build it with firefox 38 but one function we call from a custom extension no longer works.  We have the ability to open a Link in the users default browser.  If the users browser is IE it works but if it is firefox it now opens inside our branded firefox (which doesn't have any navigation buttons).   We are branding it using application.ini file (attached) reference from command line parameter.
"C:\Program Files (x86)\CompanyName\Eagle_dev\eagleclient.exe"  -app "C:\PROGRA~2\CompanyName\EAGLE_~1\browser\APPLICATION.INI"

see Bug 723493 for background on that.  This allows our custom firefox version to co-exist with whatever version of firefox the user may have installed.
We use the technique described here for opening external URL
https://developer.mozilla.org/en-US/docs/Opening_a_Link_in_the_Default_Browser





Actual results:

Link opens in branded version of firefox


Expected results:

Should have opened in non branded version of firefox.  This worked in 24.5.  no longer works in 38 (ESR)

	Loic
	Comment 1 • 9 years ago

Do you have reduced STR to test locally (on my side, with a custom profile and application.ini e.g.)?

	Don Kleppinger Reporter
	Comment 3 • 9 years ago

Here's how to reproduce

1) copy attached application.ini to 
C:\Program Files (x86)\Mozilla Firefox\browser\

2) Create a shortcut on your desktop with this in it
"C:\Program Files (x86)\Mozilla Firefox\firefox.exe" -app "C:\Program Files (x86)\Mozilla Firefox\browser\application.ini"

3) open the shortcut 
4) save the attached xpi extenstion to your desktop and drag it onto the branded browser
5) click restart browser when it prompts you.  
6) drag and drop attached html file onto your browser
7) click link
8) this is what is strange.  It works first now if you had clicked the restart link but if you just closed the browser and then restarted or close the browser now and restart it no longer works and opens the URL in the branded firefox from now on.

	Don Kleppinger Reporter
	Comment 4 • 9 years ago

Custom extension containing function to open url in default browser

	Don Kleppinger Reporter
	Comment 5 • 9 years ago

Note.
When you use that application.ini parameter it will create profile under
C:\Users\username\AppData\Roaming\SquareTwoFinancial\eagle_dev

so that's where the attached extension should get installed

	Loic
	Comment 6 • 9 years ago

(In reply to Don Kleppinger from comment #5)
> Note.
> When you use that application.ini parameter it will create profile under
> C:\Users\username\AppData\Roaming\SquareTwoFinancial\eagle_dev
> 
> so that's where the attached extension should get installed

Can I avoid this step by just (pre)creating a test profile (e.g. "test") and specifying this profile in the shortcut by:
"C:\Program Files (x86)\Mozilla Firefox\firefox.exe" -app "C:\Program Files (x86)\Mozilla Firefox\browser\application.ini" -P "test" -no-remote


NB: I added -no-remote because I want to have my current profile/browser open during testing your testcase.

	Don Kleppinger Reporter
	Comment 7 • 9 years ago

I don't think you need the -no-remote because since it is branded as a different application won't share any profile.   The profile goes under the application vendor name

	Don Kleppinger Reporter
	Comment 8 • 9 years ago

You need the extension.  That's where the code is that opens the url using defined mimetype to open using users default browser.
If you drag the extension onto the browser after it is open it will install in the applications profile directory. This won't install it into your existing firefox or interfere with your existing firefox installation in any way.  So I don't think specifying a profile or using no-remote is needed. It won't mess up your profile as long as you use the parameter to point to the attached application.ini and application.ini must reside in browser directory.  It won't work anywhere else.

	Loic
	Comment 9 • 9 years ago

I'm able to reproduce it.

I'll try to bisect to find a regression range.

	Don Kleppinger Reporter
	Comment 10 • 9 years ago

We use the ESR branch.  I just tried it on 31.7.0esr and that one doesn't exhibit the problem. 
So somewhere between the previous ESR and latest ESR (38.1ESR) it was introduced.

	Loic
	Comment 11 • 9 years ago

I found the regression range by downloading manually Nighty builds and testing them.

good=2014-08-28
bad=2014-08-29
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3be45b58fc47&tochange=d697d649c765

But I fail to identify the suspected bug.

	Alice0775 White
	Comment 12 • 9 years ago

If the default browser is IE of (Chrome), you can reproduce the problem?

	Alice0775 White
	Comment 13 • 9 years ago

s/of (Chrome)/or Chrome/

	Don Kleppinger Reporter
	Comment 14 • 9 years ago

It works fine if default browser if IE or Chrome.  It fails when default browser is Firefox.   It opens in the Customized branded version of firefox instead of the users regular firefox installation.

	Alice0775 White
	Comment 16 • 9 years ago

I can reproduce now.

	Alice0775 White
	Comment 17 • 9 years ago

[Tracking Requested - why for this release]: it is worth to fix ESR38

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fd00d686183f&tochange=1910714b56c6

Regressed by: Bug 1023941

	Don Kleppinger Reporter
	Comment 18 • 9 years ago

Thank you for the fast follow-up on this.  I'm unfamiliar with the triage process and what happens next.   What time frame am I looking at to get a fix?   Is there something I can do to patch my installation file as I am blocked from using ESR38 until I have a fix or work-around for this.

	Loic
	Comment 19 • 9 years ago

The dev who pushed the offending patch has been CC'ed so you have to wait for his reply.

	(Away) Assignee
	Comment 20 • 9 years ago

Don, thanks for the report. I'm afraid you've caught me at an extremely busy time. I'll do my best to investigate this next week but it may push out into the following week (sorry, I normally do a lot better than that).

	Don Kleppinger Reporter
	Comment 21 • 9 years ago

Thank you.   Anything you can do to expedite this will be extremely appreciated!

	(Away) Assignee
	Comment 22 • 9 years ago

Dan, Loic, Alice,

Could you please give me more explicit instructions on how to reproduce this. I tried the STR above but whenever I used the "-app" switch, the Firefox process would immediately exit, before it displayed anything. I tried to mimic this setup with "-no-remote" and another profile, but I didn't encounter the bug; the link still opened in my default browser (not the Eagle-flavoured one). Would appreciate as much detail as possible.

It would also help me debug faster if you could bisect this down to a single changeset within that push. Thanks.

	Loic
	Comment 23 • 9 years ago

STR:

1) Download a zipped (Nightly) build and unzip the folder /firefox on the desktop e.g.
2) Download and save the attached application.ini in the folder /firefox/browser
3) Create a shortcut on your desktop with this in it
"C:\Users\<user>\Desktop\firefox\firefox.exe" -app "C:\Users\<user>\Desktop\firefox\browser\application.ini"
4) Start the browser by clicking on the shortcut (it will create a new branded profile under "C:\Users\<user>\AppData\Roaming\SquareTwoFinancial\eagle_dev")
5) Download and install the attached extension in this profile
6) Restart the browser
7) Open the attached .html file and click on the link

Expected result: it should launch and start your default browser and open a new tab (on Yahoo).

Actual result: the tab is open in the current branded browser.

	Loic
	Comment 24 • 9 years ago

NB: I forgot to add I did that for every nightly build I downloaded to bisect.
No need to add -no-remote to the shortcut or modify your current defauft profile.

	Don Kleppinger Reporter
	Comment 25 • 9 years ago

updated extension with one that won't close the browser if you navigate to google

	Don Kleppinger Reporter
	Comment 26 • 9 years ago

Sorry,  The extension had code that would close the browser if you navigate to google.  The extension is used for an internal web app and the user never goes to any outside web sites, except by opening default browser, hence this bug.

	(Away) Assignee
	Comment 27 • 9 years ago

This boils down to a disagreement on the value of environment variable "XUL_APP_FILE" between firefox.exe's CRT and xul.dll's CRT. Before revision 202107, the two binaries shared the same CRT. Nowadays there are two copies of the CRT in the process.

The variable is written here (to firefox.exe's CRT): https://hg.mozilla.org/mozilla-central/file/be81b8d6fae9/browser/app/nsBrowserApp.cpp#l169. Additionally, the CRT passes along the value to the canonical storage in the OS.

Later we try to clear the variable here (from xul.dll's CRT): https://hg.mozilla.org/mozilla-central/file/be81b8d6fae9/toolkit/xre/nsAppRunner.cpp#l4188. Since xul.dll's CRT doesn't find the variable locally, it doesn't update the canonical storage in the OS. When we eventually launch the child process, the child inherits the XUL_APP_FILE, because the OS thinks the variable is still set.

As a workaround I can do SaveToEnv("XUL_APP_FILE=Dummy") before SaveToEnv("XUL_APP_FILE="). It forces xul's CRT to actually remove something, and actually talk to the OS. I don't think it's the right fix though; I bet SaveStateForAppInitiatedRestart is still broken.

	(Away) Assignee
	Comment 32 • 9 years ago

Here's an attempt at a more complete workaround: 
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/dmajor@mozilla.com-340ef566a5e5/try-win32/

	Don Kleppinger Reporter
	Comment 33 • 9 years ago

Thank you!
I was able to verify that the default browser feature worked after disabling the E10s option.  Most of the features of my extension didn't work at first including firebug but when I clicked the firebug popup message to disable E10s then my extension features worked.
Any chance you could build me a version that applies the fix to the 38esr branch?  41 has a number of incompatibilities with our customizations that we won't be able to fix in the short term.

	(Away) Assignee
	Comment 34 • 9 years ago

Yes, we'll need to get the fix into a "real" esr38 build. The try-build was just for testing the fix.

	(Away) Assignee
	Comment 35 • 9 years ago

This fix is pretty yucky and I'm not super proud of it, but I don't have any better ideas.

	Don Kleppinger Reporter
	Comment 36 • 9 years ago

Don't mean to beg, but any ETA on when I could get a ESR build with this fix?

	Pulsebot
	Comment 37 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/90c2371af31d

	(Away) Assignee
	Comment 38 • 9 years ago

(In reply to Don Kleppinger from comment #36)
> Don't mean to beg, but any ETA on when I could get a ESR build with this fix?

I'm told that this didn't make the cutoff for the ESR 38.1 build. It won't be in an official ESR release until 38.2 in about six weeks. However we can have an unofficial pre-release ESR 38.2 build available in a few days.

	(Away) Assignee
	Comment 39 • 9 years ago

Comment on attachment 8625641 [details] [diff] [review]
Wallpaper patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: feature regression, blocker for custom app writers
User impact if declined: broken new-window experience in custom app
Fix Landed on Version: 41
Risk to taking this patch (and alternatives if risky): From the code side this is a hacky fix, but from risk management perspective it is well-scoped. Regular desktop Firefox ESR users will not see a difference. There are potentially other solutions that make the code nicer (e.g. write the variable from libxul in the first place) but that would carry significantly more risk to all ESR users, including those who are unaffected by the bug.
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

	Don Kleppinger Reporter
	Comment 40 • 9 years ago

> I'm told that this didn't make the cutoff for the ESR 38.1 build. It won't
> be in an official ESR release until 38.2 in about six weeks. However we can
> have an unofficial pre-release ESR 38.2 build available in a few days.

Any chance I could get an unofficial build against the 38.1 base with this patch included?   I don't know what else might be included in 38.2 and I'd rather avoid regression testing.

	(Away) Assignee
	Comment 41 • 9 years ago

At this moment there is nothing else included in 38.2.

Just so we're on the same page: are you planning to release a product based on this test build?

	Don Kleppinger Reporter
	Comment 42 • 9 years ago

If possible.  This is for an internal enterprise application and we need to be on the latest version of the ESR for PCI certification.  We're actually planning to release next week with this defect but we were hoping for a quick follow on that will fix the bug.

	Carsten Book [:Tomcat]
	Comment 43 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/90c2371af31d

	(Away) Assignee
	Comment 44 • 9 years ago

I don't know the official stance but my gut feeling is that repackaging an unofficial build is a really bad idea. For one thing they have "Nightly" branding and icons etc -- maybe that's a non-issue if your app sets its own name and whatnot. But if you're serious about certification, I imagine a test build would be frowned upon, and you may have difficulty getting support if it breaks.

It's really unfortunate that this barely missed the cutoff and has to wait almost a full cycle, but I don't know what else we can do.

	Loic
	Comment 45 • 9 years ago

I tested with the last Windows nightly (from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/06/2015-06-30-03-02-04-mozilla-central/) and I confirm it's fixed (on Win 7): The link is open in the default browser application.

	Don Kleppinger Reporter
	Comment 46 • 9 years ago

(In reply to David Major [:dmajor] from comment #44)
Is there a nightly ESR build?  How do fixes get applied to that branch?  
You mentioned you could create a pre-release ESR 38.2.  Shouldn't that be identical to 38.1 but including the patch since you said nothing else has been added to it yet.  Yes we change the icons and hide all menu's and toolbars so the user can only interact with the page and the tabs.  What is the official release date of ESR 38.2 and is it available prior to the official release?

	(Away) Assignee
	Comment 47 • 9 years ago

Here's how the release goes: For all intents and purposes there's a single ESR38 code branch. Whenever a change is made in that branch, the automation creates a build for that change and runs our test suite on it. Those builds have unofficial branding: blue globe, the word "Nightly" in the title bar, etc. Every six weeks we do an official Firefox-branded build for release. We just made one a few days ago for 38.1.0. In six weeks we'll do a 38.2.0 build that has everything from 38.1.0 plus whatever accumulates in the ESR38 branch in that time.

You could use the per-change build for this bug as a way to verify the fix, but it wouldn't have official branding.

The current candidate build for 38.1.0 is still undergoing testing. I've spoken to release management about this bug. If other factors require that we build another 38.1.0 candidate anyway, they will consider including this fix.

	Don Kleppinger Reporter
	Comment 48 • 9 years ago

I just got this notice.  Maybe I'll get lucky.  Thanks for all your help
------------------------
Hi there Firefox ESR users, administrators, and community,
Our pre-release testing uncovered an unusually high number of stability issues caused by a third party application that we believed might impact users. We are now working on fixing this issue Firefox will be released soon.
This issue cropped up late in the beta cycle just as we were building the release candidate for 39.  Other Firefox channels, including ESR, are not significantly affected. 

However, we are not releasing Firefox 31.8.0 and 38.1.0 today. The ESR releases will be delayed, for at least a couple of days, until we are ready to ship. 

We will be testing a fix soon on the beta channel, and will let you all know when we have a date for the release.   We're glad to have caught this issue in time to prevent it affecting Firefox ESR users.

	Sylvestre Ledru [:Sylvestre]
	Comment 49 • 9 years ago

Comment on attachment 8625641 [details] [diff] [review]
Wallpaper patch

If it has no risk for our users and helps some other ESR users, let's take it.

By the way, thanks for reporting such bug, it helps to improve the quality of ESR.

	Ryan VanderMeulen [:RyanVM]
	Comment 50 • 9 years ago

Should we get this on Beta40 as well then?

	Ryan VanderMeulen [:RyanVM]
	Comment 51 • 9 years ago

https://hg.mozilla.org/releases/mozilla-esr38/rev/e827200e7369

	(Away) Assignee
	Comment 52 • 9 years ago

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #50)
> Should we get this on Beta40 as well then?

I have no preference. We don't know of any cases where it will make a difference on 40. It also won't hurt. Whatever makes your life easier.

	[SV Manager] Florin Mezei, QA (:FlorinMezei)
	Comment 53 • 9 years ago

I was able to reproduce the issue using Firefox 37.0.2 and 39.0, on Windows 7 x64, using steps from comment 23, with the addition of an additional step to close and then start again Firefox from the shortcut (as specified in comment 3 step 8). 

The issue no longer reproduced when using the latest Firefox 38 ESR tinderbox build 
- BuildID: 20150720152627 
- revision: https://hg.mozilla.org/releases/mozilla-esr38/rev/9042488f5118
- download location: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-esr38-win32/latest/

With the latest ESR the link is correctly opened in the default browser, be it Firefox or Chrome.

Marking as verified for ESR 38.

	Don Kleppinger Reporter
	Comment 54 • 9 years ago

It worked for me.  Thank you