Closed
Bug 747470
Opened 13 years ago
Closed 13 years ago
Cannot launch applications in Windows XP/Vista
Categories
(Firefox Graveyard :: Web Apps, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 15
People
(Reporter: jsmith, Assigned: TimAbraldes)
Details
(Whiteboard: [marketplace-beta+])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
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•13 years ago
|
Whiteboard: [marketplace-beta]
Assignee | ||
Comment 1•13 years ago
|
||
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
Comment 2•13 years ago
|
||
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.
Comment 3•13 years ago
|
||
Oops, this comment was meant for a different bug.
Reporter | ||
Comment 4•13 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
Comment 5•13 years ago
|
||
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
Comment 6•13 years ago
|
||
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.
Assignee | ||
Comment 7•13 years ago
|
||
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•13 years ago
|
Whiteboard: [marketplace-beta] → [marketplace-beta+]
Assignee | ||
Comment 8•13 years ago
|
||
"USE_STATIC_LIBS = 1" seems to have had no effect on the try build. I'll keep investigating...
Assignee | ||
Comment 9•13 years ago
|
||
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
Comment 10•13 years ago
|
||
(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?
Comment 11•13 years ago
|
||
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.
Updated•13 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 12•13 years ago
|
||
The previous try run failed, here's the retry:
https://tbpl.mozilla.org/?tree=Try&rev=bc3790fc9b6b
Updated•13 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 13•13 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
Assignee | ||
Comment 14•13 years ago
|
||
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)
Updated•13 years ago
|
Attachment #617972 -
Flags: review?(myk) → review+
Comment 15•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Reporter | ||
Comment 16•13 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•13 years ago
|
Status: RESOLVED → VERIFIED
Reporter | ||
Updated•13 years ago
|
Flags: in-moztrap?(jsmith)
Reporter | ||
Updated•12 years ago
|
Flags: in-moztrap?(jsmith) → in-moztrap+
Reporter | ||
Updated•12 years ago
|
QA Contact: jsmith
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•