Closed
Bug 360735
Opened 18 years ago
Closed 17 years ago
replace 'A' APIs with 'W' APIs in windows shell service
Categories
(Firefox :: Shell Integration, defect)
Tracking
()
RESOLVED
FIXED
Firefox 3 alpha7
People
(Reporter: jshin1987, Assigned: sciguyryan)
References
Details
Attachments
(1 file, 6 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
To make firefox a true Unicode application, we need to get rid of 'A' API calls throughout the tree along with 'native' API invocations of nsILocalFile.
Comment 1•18 years ago
|
||
wooohoo! This means that alot of the windows ce shunt can go away.
What about backward compat... Are W routines available everywhere we care about?
Reporter | ||
Comment 2•18 years ago
|
||
(In reply to comment #1)
> What about backward compat... Are W routines available everywhere we care
> about?
Yes, they're because we dropped the support for Win 9x/ME in gecko 1.9. Gecko 1.8.x is the last branch to support them. See, for instance, bug 330276 and bug 359808.
Assignee | ||
Comment 3•18 years ago
|
||
Should bug 330276 and bug 359808 be set as blocking this?
Reporter | ||
Comment 4•18 years ago
|
||
(In reply to comment #3)
> Should bug 330276 and bug 359808 be set as blocking this?
No. Win 9x/ME support was dropped even before they're fixed due to our switch to cairo so that we're free to use W APIs here.
Assignee | ||
Comment 5•18 years ago
|
||
I'll try and take a look at this. I guess I should also try and move all the native Windows registry API calls over to the |nsIWindowsRegKey| class while I'm at it.
Assignee: nobody → bugs
Assignee | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Assignee | ||
Comment 6•18 years ago
|
||
Patch v1
* Conversion of "A" variant API calls into the "W" variants. This patch compiles but may need some testing.
Assignee | ||
Updated•18 years ago
|
Attachment #254530 -
Flags: review?(neil)
Assignee | ||
Comment 7•18 years ago
|
||
I should note that I have testes the set as desktop background, check default browser, set default browser etc.
Updated•18 years ago
|
Attachment #254530 -
Flags: review?(neil)
Assignee | ||
Comment 8•18 years ago
|
||
Patch v1.1
* Addresses some comments from Neil via IRC.
Attachment #254530 -
Attachment is obsolete: true
Assignee | ||
Comment 9•18 years ago
|
||
Patch v1.1.1
* Same as patch Patch v1.1 with a slight alignment fix.
Attachment #254539 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Attachment #254541 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 10•18 years ago
|
||
Patch v1.1.1 (un-bitrotted)
Attachment #254541 -
Attachment is obsolete: true
Attachment #260278 -
Flags: review?(robert.bugzilla)
Attachment #254541 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 11•18 years ago
|
||
Added a header too |nsWindowsShellService.h| that we now need to avoid a breakage.
Attachment #260278 -
Attachment is obsolete: true
Attachment #265558 -
Flags: review?(robert.bugzilla)
Attachment #260278 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•17 years ago
|
Attachment #265558 -
Flags: review?(robert.bugzilla) → review?(mano)
Comment 12•17 years ago
|
||
Comment on attachment 265558 [details] [diff] [review]
Patch v1.1.2
Seth Spitzer and Robert Strong are better reviewers for this code.
Attachment #265558 -
Flags: review?(mano)
Assignee | ||
Updated•17 years ago
|
Attachment #265558 -
Flags: review?(sspitzer)
Assignee | ||
Updated•17 years ago
|
Attachment #265558 -
Flags: review?(sspitzer)
Assignee | ||
Updated•17 years ago
|
Attachment #265558 -
Flags: review?(sspitzer)
Comment 13•17 years ago
|
||
Comment on attachment 265558 [details] [diff] [review]
Patch v1.1.2
passing the review buck to robert.
Attachment #265558 -
Flags: review?(sspitzer) → review?(robert.bugzilla)
Comment 14•17 years ago
|
||
Comment on attachment 265558 [details] [diff] [review]
Patch v1.1.2
Ryan, the code looks solid - excellent patch! I'm going to compile this before r+'ing to check a couple of things and will most likely r+ later today. Thanks!
Comment 15•17 years ago
|
||
Comment on attachment 265558 [details] [diff] [review]
Patch v1.1.2
>Index: browser/components/shell/src/nsWindowsShellService.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.cpp,v
>retrieving revision 1.44
>diff -u -8 -p -r1.44 nsWindowsShellService.cpp
>--- browser/components/shell/src/nsWindowsShellService.cpp 23 Mar 2007 21:32:30 -0000 1.44
>+++ browser/components/shell/src/nsWindowsShellService.cpp 1 Apr 2007 18:25:20 -0000
>@@ -521,229 +530,247 @@ nsWindowsShellService::IsDefaultBrowser(
>...
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\http\\shell\\open"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\http\\DefaultIcon"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\https\\shell\\open"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\https\\DefaultIcon"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\ftp\\shell\\open"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\ftp\\DefaultIcon"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\gopher\\shell\\open"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\gopher\\DefaultIcon"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\FirefoxURL"));
>+ (void)DeleteRegKey(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\FirefoxHTML"));
>+
>+ (void)DeleteRegKeyDefaultValue(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\.htm"));
>+ (void)DeleteRegKeyDefaultValue(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\.html"));
>+ (void)DeleteRegKeyDefaultValue(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\.shtml"));
>+ (void)DeleteRegKeyDefaultValue(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\.xht"));
>+ (void)DeleteRegKeyDefaultValue(HKEY_CURRENT_USER,
>+ NS_LITERAL_STRING("Software\\Classes\\.xhtml"));
nit: indent consistently
Once we get a unicode aware installer (e.g. MSI) we'll be in much better shape!
r=me
Attachment #265558 -
Flags: review?(robert.bugzilla) → review+
Comment 16•17 years ago
|
||
Comment on attachment 265558 [details] [diff] [review]
Patch v1.1.2
Ryan, please don't forget to update the patch prior to landing since gopher support has been removed.
Assignee | ||
Comment 17•17 years ago
|
||
Patch v1.2
Unless this needs a sr is it OK to request checkin?
Attachment #265558 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Comment 18•17 years ago
|
||
No sr needed for browser so it is ok to checkin. Thanks!
Comment 19•17 years ago
|
||
mozilla/browser/components/shell/src/Makefile.in 1.23
mozilla/browser/components/shell/src/nsWindowsShellService.cpp 1.49
mozilla/browser/components/shell/src/nsWindowsShellService.h 1.14
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M7
Comment 20•17 years ago
|
||
Not sure why my build fails on this, only now, but anyway.
Attachment #283158 -
Flags: review?(robert.bugzilla)
Updated•17 years ago
|
Attachment #283158 -
Flags: review?(robert.bugzilla) → review+
Comment 21•17 years ago
|
||
Comment on attachment 283158 [details] [diff] [review]
fix mingw build failure
Asking approval1.9 to remove this comma.
Attachment #283158 -
Flags: approval1.9?
Comment 22•17 years ago
|
||
Comment on attachment 283158 [details] [diff] [review]
fix mingw build failure
I filed a new bug to get approval1.9 for this patch, because this is probably missed by drivers thus far, see bug 403771.
Attachment #283158 -
Attachment is obsolete: true
Attachment #283158 -
Flags: approval1.9?
You need to log in
before you can comment on or make changes to this bug.
Description
•