Closed Bug 1175039 Opened 4 years ago Closed 4 years ago

Branded firefox can't open URL in default browser anymore

Categories

(Firefox Build System :: General, defect)

38 Branch
defect
Not set

Tracking

(firefox38 wontfix, firefox38.0.5 wontfix, firefox39 wontfix, firefox40 affected, firefox41 fixed, firefox-esr31 unaffected, firefox-esr3840+ verified)

RESOLVED FIXED
mozilla41
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 --- wontfix
firefox40 --- affected
firefox41 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 40+ verified

People

(Reporter: dkleppinger, Assigned: dmajor)

References

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

Attached file application.ini
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)
Do you have reduced STR to test locally (on my side, with a custom profile and application.ini e.g.)?
Flags: needinfo?(dkleppinger)
Attached file html page for testing
Flags: needinfo?(dkleppinger)
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.
Attachment #8623144 - Attachment description: Custom extension that contains a function for opening url in default browser → html page for testing
Attached file eagle_ext.xpi (obsolete) —
Custom extension containing function to open url in default browser
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
(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.
Flags: needinfo?(dkleppinger)
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
Flags: needinfo?(dkleppinger)
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.
I'm able to reproduce it.

I'll try to bisect to find a regression range.
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.
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.
If the default browser is IE of (Chrome), you can reproduce the problem?
s/of (Chrome)/or Chrome/
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.
I can reproduce now.
[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
Blocks: 1023941
Status: UNCONFIRMED → NEW
Component: Untriaged → Build Config
Ever confirmed: true
Flags: needinfo?(dmajor)
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.
The dev who pushed the offending patch has been CC'ed so you have to wait for his reply.
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).
Thank you.   Anything you can do to expedite this will be extremely appreciated!
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.
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.
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.
Attached file eagle_ext.xpi
updated extension with one that won't close the browser if you navigate to google
Attachment #8623149 - Attachment is obsolete: true
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.
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.
Assignee: nobody → dmajor
Flags: needinfo?(dmajor)
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.
Yes, we'll need to get the fix into a "real" esr38 build. The try-build was just for testing the fix.
Attached patch Wallpaper patchSplinter Review
This fix is pretty yucky and I'm not super proud of it, but I don't have any better ideas.
Attachment #8625641 - Flags: review?(benjamin)
Don't mean to beg, but any ETA on when I could get a ESR build with this fix?
Attachment #8625641 - Flags: review?(benjamin) → review?(ted)
Attachment #8625641 - Flags: review?(ted) → review+
(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.
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.
Attachment #8625641 - Flags: approval-mozilla-esr38?
> 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.
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?
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.
https://hg.mozilla.org/mozilla-central/rev/90c2371af31d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
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.
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.
(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?
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.
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.
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.
Attachment #8625641 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Should we get this on Beta40 as well then?
(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.
Flags: needinfo?(dmajor)
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.
It worked for me.  Thank you
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.