Implement "-app" parameter support for xulrunner-stub

RESOLVED FIXED in mozilla1.9.1

Status

Toolkit Graveyard
XULRunner
--
major
RESOLVED FIXED
10 years ago
2 years ago

People

(Reporter: Ronald Schouten, Assigned: Antonio Gomes (tonikitoo))

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.1
fixed1.9.1

Details

Attachments

(2 attachments, 8 obsolete attachments)

4.95 KB, patch
Benjamin Smedberg
: review+
Benjamin Smedberg
: approval1.9.1+
Details | Diff | Splinter Review
613 bytes, patch
mfinkle
: review+
Benjamin Smedberg
: approval1.9.1+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b5) Gecko/2008050509 Firefox/3.0b5
Build Identifier: Prism for Firefox 0.2

I'm not sure where to report this bug as it could be a problem with the Prism extension or how Firefox is packed for Ubuntu. Starting here, but please advise.

I just upgraded to Ubuntu 8.04 and installed the Prism extension from Mozilla Add-ons. I am able to create desktop launchers but double-clicking them opens Firefox to its homepage instead of Prism with the desired web application.

I have tried this with the following URLs:

http://mail.google.com/mail/#inbox
http://www.google.com/reader/view/#overview-page

Running the launchers exec command from the terminal produces the same result with no errors reported.

Reproducible: Always

Steps to Reproduce:
1. Install Prism for Firefox 0.2 from Mozilla Add-ons
2. Create desktop launcher for site of choosing
3. Double click on desktop launcher
Actual Results:  
Firefox opens to homepage

Expected Results:  
Prism opens with desired web application

BTW, I'm not sure what version of Prism I have installed. According to the Add-ons site it is "Prism for Firefox 0.2" but this seems a bit old and I recall that the current version is 0.9.

Comment 1

10 years ago
I confirm this bug. The Prism Extension v0.2 is practically unusable on Linux because of it. I have FF 3 Beta 5 on Ubuntu 8.04.

Comment 2

10 years ago
This appears to be an issue related to the -app argument somehow simply not working. I first noticed it when trying to use firefox instead of xulrunner to start a xul application:

http://developer.mozilla.org/en/docs/XULRunner_tips#Using_Firefox_3_to_run_XULRunner_applications

I followed those instructions, and instead of opening my app window (as xulrunner would), I found that firefox simply loaded its usual window and start page.

Then, upon starting a Prism shortcut (to gmail), it exhibited the same behavior -- I got the default firefox window and start page instead.

I am also running Firefox 3 on Ubuntu 8.04
Can someone install a Mozilla build of Firefox 3 to see if there is a build configuration issue?

Comment 4

10 years ago
Installed 3.0rc2 from http://www.mozilla.com/en-US/firefox/all-beta.html

Problem does not exist, -app flag starts up the expected xul application.

Comment 5

10 years ago
So this bug is invalid, right?
(Reporter)

Comment 6

10 years ago
Most likely but I'm not sure, as it depends on what the Ubuntu developers have to say.

For more information see: https://bugs.launchpad.net/ubuntu/+source/ubufox/+bug/241787

Comment 7

10 years ago
This bug happens on all --with-libxul-sdk builds of firefox. Point is that the firefox binary (aka xulrunner-stub) doesnt provide a fully fledged xulrunner command line interface.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 8

10 years ago
I can confirm this bug on openSUSE 11.0 KDE4. Firefox version shiped with distribution: Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.0) Gecko/2008061600 SUSE/3.0-0.2 Firefox/3.0

Clicking on short cut made by "Prism for Firefox" executes firefox homepage. It is not fixed also in Prism for Firefox 0.9.1 (Experimental)  [ refractor-linux.xpi ]

However Prism works as an standalone app, not as addon, as expected. (Prism 0.9.1 can by found here http://browsing.justdiscourse.com/2008/07/10/mozilla-prism-091-experimental-now-available/ )

Comment 9

10 years ago
Oh, I forgot. From #Prism IRC Conversation: "need to fix stub, it doesn't handle the -app flag. firefox -app doesn't work on Linux"
(Reporter)

Comment 10

10 years ago
Between Alexander's and Piotr's comments I'm not sure where the fix is. Can someone in the know make a clear comment on this issue?
Some Linux distros use Firefox-on-XULRunner. Which means that the firefox binary is actually the xulrunner-stub. The xulrunner-stub doesn't have the commandline handler for the -app flag.

The fix should be to add the handler to the xulrunner-stub.

What do you think, Benjamin?

Comment 12

10 years ago
That's acceptable.
Thinking more about this, changing xulrunner-stub is _not_ the right answer. Realize that "firefox -app" means "let this xulapp use the libxul runtime that powers firefox" and, further, firefox-on-xulrunner doesn't have a libxul of it's own.

Therefore, we should try to fix this bug by _not_ using "firefox -app" when Friefox has no libxul, but instead use the XULRunner runtime that Firefox itself is using.

Using "XpcomLib" or "GreD" could get us the path to the xulrunner runtime.

Updated

10 years ago
Duplicate of this bug: 432433

Comment 15

9 years ago
The problem is because the firefox binary on ubuntu (and Suse I guess) doesn't accept the -app parameter.

Now this link[1] says that under linux you should use the xulrunner binary instead of the firefox binary to launch XULRunner applications.

So, in order to fix this, shouldn't the plugin create the link using the firefox binary for windows and using the xulrunner one for linux?

[1] https://developer.mozilla.org/en/XULRunner_tips#Using_Firefox_3_to_run_XULRunner_applications

Comment 16

9 years ago
Starting from comment 13 we could go further and provide a |XulAppCmd| that would allow extensions to gather the right xulrunner compatible command in a transparent fashion.

For example, when firefox is build --with-libxul-sdk it would point to $gre_dir/xulrunner ... otherwise $appdir/$appname.

Comment 17

9 years ago
I'm happy to go with Mark's approach (i.e. using the shared XULRunner). Furthermore I would be delighted if someone with a Linux focus were willing to take a crack at this. I'm proposing it as blocking to Prism 1.0 and we want to get that out ASAP. Mark and I both have tons of work for 1.0 already. Come on Linux people, let's make Prism great on Linux as well!
Flags: blocking1.0?(mark.finkle)
(Assignee)

Updated

9 years ago
Assignee: nobody → tonikitoo
Flags: blocking1.0?(mark.finkle) → blocking1.0+

Updated

9 years ago
Duplicate of this bug: 476322
(Assignee)

Comment 19

9 years ago
*finally* working effectively on it.

approach: trying to make xulrunner-stup to support -app ... as per mark suggestion on irc.

uploading a patch soon.
Status: NEW → ASSIGNED
(Assignee)

Comment 20

9 years ago
Created attachment 366583 [details] [diff] [review]
patch 0.1 - add -app support to xulrunner-stub (WIP)

First working version on linux. It has some issues I am trying to fix/improve, but the core of the idea would be it.

It works fine on Linux, I will test on other platforms (I have only WinXP soon).


@Mark:

1) should we change bug title to reflect to what the fix really is ?

2) is that the right approach ? (keep in mind if it looks hacky it is because i started yesterday at around 11:47 PM :))
(Assignee)

Comment 21

9 years ago
Created attachment 366590 [details] [diff] [review]
patch 0.2 - add -app support to xulrunner-stub

same patch as previous 0.1, but functional.
Attachment #366583 - Attachment is obsolete: true
Attachment #366590 - Flags: review?(mark.finkle)
(Assignee)

Comment 22

9 years ago
Created attachment 367490 [details] [diff] [review]
patch 0.21 - add -app support to xulrunner-stub   

same patch as 0.2 but making changes ifdef'ed to XP_UNIX.
Attachment #366590 - Attachment is obsolete: true
Attachment #366590 - Flags: review?(mark.finkle)
(Assignee)

Comment 23

9 years ago
so patch works fine if launching:

1) from xulrunner-stub itself (in .../browser/dist/bin/xulrunner)

$ ./xulrunner-stub -app /home/agomes/.mozilla/firefox/uu0nc5ri.prism/extensions/refractor@developer.mozilla.org/prism/application.ini  -override /home/agomes/.webapps/moz@prism.app/override.ini -webapp moz@prism.app

2) from firefox-xulrunner-stub  (in .../browser/dist/bin/)

/firefox -app /home/agomes/.mozilla/firefox/uu0nc5ri.prism/extensions/refractor@developer.mozilla.org/prism/application.ini  -override /home/agomes/.webapps/moz@prism.app/override.ini -webapp moz@prism.app

3) from clicking the webapp icon (e.g. in ~/Desktop)

Exec="/home/agomes/mozilla/objdir/browser/dist/bin/firefox" -app "/home/agomes/.mozilla/firefox/uu0nc5ri.prism/extensions/refracto
r@developer.mozilla.org/prism/application.ini" -override "/home/agomes/.webapps/moz@prism.app/override.ini" -webapp moz@prism.app


one thing that has to be pointed out is: 

1) patch does add support for "-app" parameter only, so other ones that show up above , including "-override" and "-webapp" are being ignored by the stub.
(Assignee)

Updated

9 years ago
Attachment #367490 - Flags: review?(mark.finkle)
Attachment #367490 - Flags: review?(mark.finkle) → review-
Comment on attachment 367490 [details] [diff] [review]
patch 0.21 - add -app support to xulrunner-stub   

Overall, this looks pretty good. One thing I would like to do is move the "--app" out of the Linux-only block and make it available for all platforms.

We already have a use for this in Fennec (Windows Mobile).
(Assignee)

Comment 25

9 years ago
Created attachment 368827 [details] [diff] [review]
patch 0.3 - add -app support to xulrunner-stub

same patch as 0.21 but not ifdef'ed to Linux.

highlights:

* I have not tested it on Win32/CE or MAC, but on linux only. I suppose these are the platforms to be considered here (?).

* the Mac change has to be reviewed and tested. I can test on windows.

* my bigger concern: scenario

0) pre-conditions: 
- patch is applied (xulrunner stub supports -app parameter).
- user has prism addon installed.
1) user goes to any website (www.xyz.ab) and make a webapp off of it.
2) user does not close FF, and try to run the webapp by clicking on the Prism Icon generated on desktop. Although "-app" is supported, it will keep opening a FF window, as if user has one firefox opened and type 'firefox' again in the terminal (just a new window is created not another instance of Firefox on its own process). 

if we close firefox xulrunner_stub + "-app" works fine.

Mark, ^
Attachment #367490 - Attachment is obsolete: true
Attachment #368827 - Flags: review?(mark.finkle)
(Assignee)

Comment 26

9 years ago
as bug scope is now bigger than original goal, I am changing it accordingly.

was: "prism extension creates desktop launcher but opens firefox".
Assignee: tonikitoo → nobody
Component: Prism → XULRunner
Flags: blocking1.0+
OS: Linux → All
Product: Mozilla Labs → Toolkit
QA Contact: prism → xulrunner
Hardware: x86 → All
Summary: Prism Extension creates desktop launcher but opens Firefox → Implement "-app" parameter support for xulrunner-stub
Target Milestone: -- → ---
Version: unspecified → Trunk
(Assignee)

Updated

9 years ago
Attachment #368827 - Flags: review?(mark.finkle) → review-
(Assignee)

Comment 27

9 years ago
Comment on attachment 368827 [details] [diff] [review]
patch 0.3 - add -app support to xulrunner-stub

as per discussed on IRC:

1) wince does not work w/ this. Brad helped me to fix this.

2) there should be no link to nspr in stub.

3) this stub will need a more robust way to find the greDir.

new patch coming...
(Assignee)

Updated

9 years ago
Assignee: nobody → tonikitoo
(Assignee)

Comment 28

9 years ago
Created attachment 369827 [details] [diff] [review]
patch 0.4 - add -app support to xulrunner-stub
Attachment #369827 - Flags: review?
(Assignee)

Comment 29

9 years ago
Comment on attachment 369827 [details] [diff] [review]
patch 0.4 - add -app support to xulrunner-stub

* patch removes NSPR dep introduced in previous patch. It is wrong according to GRE docs.

Open questions:

-> @mfinkle: we find GRE in two ways here:

1) looks for a xulrunner dir in the same folder of application.in
2) find the xulrunner dir installed/registered in the system through GRE_GetGREPathWithProperties call

what can be improved ?

-> need brad (or some other wince lover) to test on that platform.
Attachment #369827 - Flags: review? → review?(mark.finkle)
(Assignee)

Updated

9 years ago
Attachment #368827 - Attachment is obsolete: true
(Assignee)

Comment 30

9 years ago
Created attachment 370647 [details] [diff] [review]
patch 0.5 - add -app support to xulrunner-stub

as per irc discussion, patch 0.5 now postpones the use of -app parameter (if passed in) to after greDir initial checks. 

Reason: it is desired to keep the current way of looking for the greDir, avoiding making it relative to the specified application.ini path.

ps: tested on linux only so far.
Attachment #369827 - Attachment is obsolete: true
Attachment #370647 - Flags: review?(mark.finkle)
Attachment #369827 - Flags: review?(mark.finkle)
Attachment #370647 - Flags: review?(mark.finkle) → review+
Comment on attachment 370647 [details] [diff] [review]
patch 0.5 - add -app support to xulrunner-stub


>+static nsresult SetEnvironmentVariable(const char* aEnvVar, const char* aValue)

I worry about this function name since it is the same as a #define in the windows headers. Could we rename to | SetEnv | ?

>+
>+/**
>+ * Return true if |greDir| is a valid GRE directory.

   * Return true if a given folder exists.

(this function is not limited to gre folders)

>+ */
>+static PRBool FolderExists(const char* greDir)

const char* aDir

>+{
>+#ifdef WINCE
>+  wchar_t wideGreDir[MAX_PATH];

wchar_t wideDir[MAX_PATH];

>+#else
>+  return access(greDir, R_OK) == 0;
>+#endif
>+
>+  return 0;

This | return 0; | is not needed. The #else handles it.

>+}
>+
>+static const char* GetRealPath(const char* appDataFile)
>+{
>+  char result[MAXPATHLEN];

I guess the the static function makes the data inside static too. I was a little worried the buffer was being freed when the function returned.

r=me with these comments addressed
shouldn't the functionality of SetEnvironmentVariable be provided by PR_SetEnv?  If not, lets fix PR_SetEnv.
(Assignee)

Comment 33

9 years ago
(In reply to comment #32)
> shouldn't the functionality of SetEnvironmentVariable be provided by PR_SetEnv?
>  If not, lets fix PR_SetEnv.

brad, according to https://developer.mozilla.org/En/GRE , section "Statically link to xpcomglue.lib (the "standalone glue") " XR stub can not link to NSPR as it is statically getting linked against the glue. that is why we are implementing our own setenv routine.
(Assignee)

Comment 34

9 years ago
Created attachment 370876 [details] [diff] [review]
patch 0.6 - add -app support to xulrunner-st

addressed mark's comment + fixed problem w/ the way it was looking for libxpcom in the same dir as the stub.
Attachment #370647 - Attachment is obsolete: true
Attachment #370876 - Flags: review?(mark.finkle)
(Assignee)

Comment 35

9 years ago
fixed error in patch 0.6:

* new patch:

+      if (!greFound) {
+        lastSlash = strrchr(iniPath, PATH_SEPARATOR_CHAR);
+        if (!lastSlash)
+          return 1;
+
+        *(++lastSlash) = '\0';
+
+        snprintf(greDir, sizeof(greDir), "%s" XPCOM_DLL, iniPath);
+        greFound = FolderExists(greDir);
+      }

* previous patch:

+      // We have a valid application.ini path passed in to the -app parameter 
+      // but not yet a valid greDir, so lets look for it also on the same folder
+      // as the stub.
+      if (!greFound) {
+        snprintf(greDir, sizeof(greDir), "%s" XPCOM_DLL, iniPath);
+        greFound = FolderExists(greDir);
+      }
Attachment #370876 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 36

9 years ago
Created attachment 370959 [details] [diff] [review]
patch 0.7 - add -app support to xulrunner-stub 

more xp-like IsArg implementation. from irc:

<blassey> fine by me
<mfinkle> yes, looks good
Attachment #370876 - Attachment is obsolete: true
Attachment #370959 - Flags: review?(mark.finkle)
Attachment #370959 - Flags: review?(mark.finkle)
Attachment #370959 - Flags: review?(benjamin)
Attachment #370959 - Flags: review+
Comment on attachment 370959 [details] [diff] [review]
patch 0.7 - add -app support to xulrunner-stub 

Benjamin - The only odd thing you might notice in this patch (hopefully) is the second look for the greDir.

I had Antonio only do this check if the -app param was used so it didn't change the old behavior of the stub. We need to allow for the stub to be _in_ the greDir (instead of above it as the first check assumes) for WinCE at least.
tracking-fennec: --- → 1.0a1-wm+

Comment 38

9 years ago
Comment on attachment 370959 [details] [diff] [review]
patch 0.7 - add -app support to xulrunner-stub 

>diff --git a/xulrunner/stub/nsXULStub.cpp b/xulrunner/stub/nsXULStub.cpp

This file uses nsWindowsWMain.cpp. I think this means that all Windows char* in this file are UTF8, not native charset. If this is the case, several i18n bugs were already present in the Windows (desktop) version, but are being made more explicitly bad in the WINCE version.

If this is *not* the case, we should really make it clear which char* are the UTF8-munged kind and which are truly native, probably using typedefs.

>+static PRBool FolderExists(const char* aDir)
>+{
>+#ifdef WINCE
>+  wchar_t wideDir[MAX_PATH];
>+  MultiByteToWideChar(CP_ACP, 0, aDir, -1, wideDir, MAX_PATH);

CP_UTF8

>+  DWORD fileAttrs = GetFileAttributesW(wideDir);
>+  return fileAttrs != INVALID_FILE_ATTRIBUTES;
>+#else
>+  return access(aDir, R_OK) == 0;

Should be _waccess

>+static nsresult GetRealPath(const char* appDataFile, char* *aResult)
>+{
>+#ifdef XP_WIN
>+  wchar_t wAppDataFile[MAX_PATH];
>+  wchar_t wIniPath[MAX_PATH];
>+  MultiByteToWideChar(CP_ACP, 0, appDataFile, -1, wAppDataFile, MAX_PATH);

CP_UTF8

> class AutoAppData
> {
> public:
>   AutoAppData(nsILocalFile* aINIFile) : mAppData(nsnull) {
>     nsresult rv = XRE_CreateAppData(aINIFile, &mAppData);
>     if (NS_FAILED(rv))
>@@ -186,17 +241,16 @@ main(int argc, char **argv)
> 
> #ifdef XP_WIN
>   wchar_t wide_path[MAX_PATH];
>   if (!::GetModuleFileNameW(NULL, wide_path, MAX_PATH))
>     return 1;
> 
>   WideCharToMultiByte(CP_ACP, 0, wide_path,-1,
> 		      iniPath, MAX_PATH, NULL, NULL);

I think this should be CP_UTF8 also

>+      char *result = (char*) calloc(sizeof(char), MAXPATHLEN);
>+      memset(result, 0, MAXPATHLEN);

It shouldn't be necessary to calloc() and memset this data: calloc initializes to 0
Attachment #370959 - Flags: review?(benjamin) → review-
(In reply to comment #38)
> (From update of attachment 370959 [details] [diff] [review])
> >diff --git a/xulrunner/stub/nsXULStub.cpp b/xulrunner/stub/nsXULStub.cpp
> >+  DWORD fileAttrs = GetFileAttributesW(wideDir);
> >+  return fileAttrs != INVALID_FILE_ATTRIBUTES;
> >+#else
> >+  return access(aDir, R_OK) == 0;
> 
> Should be _waccess

access -> _waccess  or we should use _waccess in the WINCE block? I'm pretty sure _waccess is not supported on WinCE

Comment 40

9 years ago
Windows desktop should not use access() on a UTF8 char*. It should either use the GetFileAttributesW code above, or should use _waccess.
(Assignee)

Comment 41

9 years ago
Created attachment 372700 [details] [diff] [review]
patch 0.8 - add -app support to xulrunner-stub

fixed benjamin's comments.
Attachment #370959 - Attachment is obsolete: true
Attachment #372700 - Flags: review?(benjamin)
(Assignee)

Comment 42

9 years ago
> >+static PRBool FolderExists(const char* aDir)
> >+{
> >+#ifdef WINCE
> >+  wchar_t wideDir[MAX_PATH];
> >+  MultiByteToWideChar(CP_ACP, 0, aDir, -1, wideDir, MAX_PATH);
> 
> CP_UTF8

done.

> >+  DWORD fileAttrs = GetFileAttributesW(wideDir);
> >+  return fileAttrs != INVALID_FILE_ATTRIBUTES;
> >+#else
> >+  return access(aDir, R_OK) == 0;
> 
> Should be _waccess

instead of #WINCE it was changed to #WINXP, so both will be using GetFileAttributesW as suggested.

> >+static nsresult GetRealPath(const char* appDataFile, char* *aResult)
> >+{
> >+#ifdef XP_WIN
> >+  wchar_t wAppDataFile[MAX_PATH];
> >+  wchar_t wIniPath[MAX_PATH];
> >+  MultiByteToWideChar(CP_ACP, 0, appDataFile, -1, wAppDataFile, MAX_PATH);
> 
> CP_UTF8

done.

> > public:
> >   AutoAppData(nsILocalFile* aINIFile) : mAppData(nsnull) {
> >     nsresult rv = XRE_CreateAppData(aINIFile, &mAppData);
> >     if (NS_FAILED(rv))
> >@@ -186,17 +241,16 @@ main(int argc, char **argv)
> > 
> > #ifdef XP_WIN
> >   wchar_t wide_path[MAX_PATH];
> >   if (!::GetModuleFileNameW(NULL, wide_path, MAX_PATH))
> >     return 1;
> > 
> >   WideCharToMultiByte(CP_ACP, 0, wide_path,-1,
> > 		      iniPath, MAX_PATH, NULL, NULL);
> 
> I think this should be CP_UTF8 also

done.

> >+      char *result = (char*) calloc(sizeof(char), MAXPATHLEN);
> >+      memset(result, 0, MAXPATHLEN);
> 
> It shouldn't be necessary to calloc() and memset this data: calloc initializes
> to 0

done.
Comment on attachment 372700 [details] [diff] [review]
patch 0.8 - add -app support to xulrunner-stub

>+static PRBool FolderExists(const char* aDir)
>+{
>+#ifdef WINXP
>+  wchar_t wideDir[MAX_PATH];
>+  MultiByteToWideChar(CP_UTF8, 0, aDir, -1, wideDir, MAX_PATH);
>+  DWORD fileAttrs = GetFileAttributesW(wideDir);
>+  return fileAttrs != INVALID_FILE_ATTRIBUTES;
>+#else
>+  return access(aDir, R_OK) == 0;
>+#endif
>+}

WINCE insted of WINXP maybe.

Comment 44

9 years ago
Comment on attachment 372700 [details] [diff] [review]
patch 0.8 - add -app support to xulrunner-stub

>diff --git a/xulrunner/stub/nsXULStub.cpp b/xulrunner/stub/nsXULStub.cpp

>+#ifdef WINXP

I think you meant XP_WIN here?

r=me with that fixed
Attachment #372700 - Flags: review?(benjamin) → review+
pushed http://hg.mozilla.org/mozilla-central/rev/fc10ea0d9806
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Flags: wanted1.9.1?
(Assignee)

Comment 46

9 years ago
marking "wanted 191 ?" 

it'd be nice to have it on 1.9.1, once fennec wince AND prism 1.0 would be using it.
It's also a relatively safe XULRunner stub only change. It would be a benefit to Linux distros that use FF+XR builds.
Created attachment 373396 [details] [diff] [review]
Mac bustage fix

Gavin discovered bustage on Mac. Here's the fix.
Attachment #373396 - Flags: review+

Updated

9 years ago
Duplicate of this bug: 492206
Attachment #372700 - Flags: approval1.9.1?
Attachment #373396 - Flags: approval1.9.1?
I'd like to get this patch on 1.9.1 so Firefox on Linux, which uses XULRunner as a runtime, can support the same -app functionality as Firefox on Mac and Windows. This patch is local to xulrunner-stub and does not affect Firefox directly, or at all on Windows and Mac.

On Linux, this patch only affects using Firefox as a runtime to launch xulapps. The only risk is that the patch is in the startup path of Firefox on Linux, but this patch has baked for a while on trunk with no ill affects.
Benjamin: thoughts on comment 50? I'm inclined to take it if you feel that it's safe.

Updated

9 years ago
Attachment #373396 - Flags: approval1.9.1? → approval1.9.1+

Updated

9 years ago
Attachment #372700 - Flags: approval1.9.1? → approval1.9.1+

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [needs 191 landing]
It attachment 373396 [details] [diff] [review] needed on 1.9.1?
1.9.1 includes <CFBundle.h> instead of <CoreFoundation/CoreFoundation.h>.
(In reply to comment #53)
> It attachment 373396 [details] [diff] [review] needed on 1.9.1?
> 1.9.1 includes <CFBundle.h> instead of <CoreFoundation/CoreFoundation.h>.

Karl - That is from bug 482277 which is removing carbon code. That bug/patch does not seem to be going to 191, only trunk. So <CFBundle.h> should stay in 191.

Comment 55

9 years ago
Related bug in launchpad: https://bugs.launchpad.net/ubuntu/+source/xulrunner/+bug/337176
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/86b8356da552
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/03b37937512a
Keywords: checkin-needed → fixed1.9.1
Whiteboard: [needs 191 landing]
Target Milestone: --- → mozilla1.9.1

Updated

9 years ago
Duplicate of this bug: 501678
Flags: wanted1.9.1?
Duplicate of this bug: 504197

Updated

7 years ago
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.