Cannot launch applications in Windows XP/Vista

VERIFIED FIXED in Firefox 15

Status

Firefox Graveyard
Web Apps
--
critical
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: jsmith, Assigned: TimAbraldes)

Tracking

14 Branch
Firefox 15
x86
Windows XP
Bug Flags:
in-moztrap +

Details

(Whiteboard: [marketplace-beta+])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Steps:

1. Go to https://apps.mozillalabs.com/appdir/
2. Install an application
3. Launch an application

Expected:

The application should start up in a chromeless window.

Actual:

The command prompt appears, but an error dialog is shown saying:

BarFight.exe - Unable to Locate Component

The application has failed to start because MSVCR100.dll was not found. Re-installing the application may fix this problem.
(Reporter)

Updated

5 years ago
Whiteboard: [marketplace-beta]
Verified with Jason that the issue goes away if msvcr100.dll is copied from the Firefox installation dir to the webapp's installation dir.

One option for fixing this issue is to copy msvcr100.dll from the Firefox installation dir to webapp installation dirs when installing the webapps.

However, the right way to fix this is probably to statically link against the CRT.  I'll investigate that approach and see if I hit any roadblocks.
Assignee: nobody → tabraldes
Status: NEW → ASSIGNED
(Reporter)

Updated

5 years ago
Blocks: 731054
I don't think this is a valid bug. Web apps change all the time, so we don't really know or care what the version or publisher information is. I believe that the current behavior is most correct.
Oops, this comment was meant for a different bug.
(Reporter)

Comment 4

5 years ago
Also getting a reproduction of this issue on Windows Vista.
Summary: Cannot launch applications in Windows XP → Cannot launch applications in Windows XP/Vista
Presumably it'd reproduce on any system < Windows 8 (which I'd guess ships this CRT) which hasn't had the CRT installed already (by installing VC2010 or something else that installs it as a dependency).

You can link your stub with USE_STATIC_LIBS = 1 in the Makefile, or you can copy the runtime files over. If you do the latter, don't hardcode the names, pull them from the build system somewhere, like, say:
http://mxr.mozilla.org/mozilla-central/source/build/win32/Makefile.in#72
We should just add USE_STATIC_LIBS to /webapprt/win/Makefile.in. I thought I mentioned that somewhere but unless it causes some weird build bustage it is the correct way to do this.
Created attachment 617685 [details] [diff] [review]
Patch v1 - Add USE_STATIC_LIBS to webapprt makefile

I tried adding "USE_STATIC_LIBS = 1" on Friday but it seemed to have no effect on my local builds (the command line still showed "/MDd" and the built executable still tried to load msvcr100.dll at runtime).  Running the change through try; hopefully there was just something weird about my local builds.

https://tbpl.mozilla.org/?tree=Try&rev=e4704a8b0013
(Reporter)

Updated

5 years ago
Whiteboard: [marketplace-beta] → [marketplace-beta+]
"USE_STATIC_LIBS = 1" seems to have had no effect on the try build.  I'll keep investigating...
Created attachment 617972 [details] [diff] [review]
Patch v2 - include correct makefile

This makefile included "$(top_srcdir)/config/config.mk" instead of including "$(DEPTH)/config/autoconf.mk"

Correcting this issue made the "USE_STATIC_LIBS = 1" line have the intended effect.

Running this through try:
https://tbpl.mozilla.org/?tree=Try&rev=bb42284bf942
Attachment #617685 - Attachment is obsolete: true
(In reply to Tim Abraldes from comment #9)
> This makefile included "$(top_srcdir)/config/config.mk" instead of including
> "$(DEPTH)/config/autoconf.mk"

It used to include both, but I changed it to include only the former:

https://github.com/michaelrhanson/mozilla-central/commit/145a776343b377ad5a1e62c3564f1f052ffa241d#diff-19

Because the former includes the latter:

http://mxr.mozilla.org/mozilla-central/source/config/config.mk#59

So I don't understand why this change has the desired effect.  Also, are you sure we don't need anything in $(top_srcdir)/config/config.mk?
The correct format for a makefile is:

DEPTH = .. (etc)

include $(DEPTH)/config/autoconf.mk

# Set variables like PROGRAM/CPPSRCS/etc

include $(topsrcdir)/config/config.mk # this is optional, only necessary if you need to modify the variables it sets. rules.mk automatically includes config.mk

# override variables here

include $(topsrcdir)/config/rules.mk # required

# override rules here

So tim's patch is correct and necessary, because you are setting variables (in particular USE_STATIC_LIBS) after including config.mk, which means it will have no effect.
Keywords: checkin-needed
The previous try run failed, here's the retry:
https://tbpl.mozilla.org/?tree=Try&rev=bc3790fc9b6b
Keywords: checkin-needed
(Reporter)

Comment 13

5 years ago
Doing some bare bones testing with try build in comment 12 revealed the following behavior on XP/Vista:

- I can launch applications that were installed with the try build
- I cannot launch applications that were installed with the other nightly build that produced this bug
Comment on attachment 617972 [details] [diff] [review]
Patch v2 - include correct makefile

(In reply to Jason Smith from comment #13)
> Doing some bare bones testing with try build in comment 12 revealed the
> following behavior on XP/Vista:
> 
> - I can launch applications that were installed with the try build

Hooray!

> - I cannot launch applications that were installed with the other nightly
> build that produced this bug

This is expected.  Any broken app installations will have to be reinstalled.
Attachment #617972 - Flags: review?(myk)
Attachment #617972 - Flags: review?(myk) → review+
https://hg.mozilla.org/mozilla-central/rev/b952bb042f12
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
(Reporter)

Comment 16

5 years ago
Felipe - This lands in tomorrow's nightly build, correct? (Double checking, cause this isn't working on today's nightly build)
(Reporter)

Updated

5 years ago
Status: RESOLVED → VERIFIED
(Reporter)

Updated

5 years ago
No longer blocks: 731054
(Reporter)

Updated

5 years ago
Flags: in-moztrap?(jsmith)
(Reporter)

Updated

5 years ago
Flags: in-moztrap?(jsmith) → in-moztrap+
(Reporter)

Updated

5 years ago
QA Contact: jsmith
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.