Closed Bug 360735 Opened 15 years ago Closed 14 years ago

replace 'A' APIs with 'W' APIs in windows shell service

Categories

(Firefox :: Shell Integration, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 alpha7

People

(Reporter: jshin1987, Assigned: sciguyryan)

References

Details

Attachments

(1 file, 6 obsolete files)

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.
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?
(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.



Should bug 330276 and bug 359808 be set as blocking this?
(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. 
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
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Attached patch Patch v1 (obsolete) — Splinter Review
Patch v1

* Conversion of "A" variant API calls into the "W" variants. This patch compiles but may need some testing.
Attachment #254530 - Flags: review?(neil)
I should note that I have testes the set as desktop background, check default browser, set default browser etc.
Attachment #254530 - Flags: review?(neil)
Attached patch Patch v1.1 (obsolete) — Splinter Review
Patch v1.1

* Addresses some comments from Neil via IRC.
Attachment #254530 - Attachment is obsolete: true
Attached patch Patch v1.1.1 (obsolete) — Splinter Review
Patch v1.1.1

* Same as patch Patch v1.1 with a slight alignment fix.
Attachment #254539 - Attachment is obsolete: true
Attachment #254541 - Flags: review?(robert.bugzilla)
Attached patch Patch v1.1.1 (mark 2) (obsolete) — Splinter Review
Patch v1.1.1 (un-bitrotted)
Attachment #254541 - Attachment is obsolete: true
Attachment #260278 - Flags: review?(robert.bugzilla)
Attachment #254541 - Flags: review?(robert.bugzilla)
Attached patch Patch v1.1.2 (obsolete) — Splinter Review
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)
Attachment #265558 - Flags: review?(robert.bugzilla) → review?(mano)
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)
Attachment #265558 - Flags: review?(sspitzer)
Attachment #265558 - Flags: review?(sspitzer)
Attachment #265558 - Flags: review?(sspitzer)
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 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 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 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.
Attached patch Patch v1.2Splinter Review
Patch v1.2

Unless this needs a sr is it OK to request checkin?
Attachment #265558 - Attachment is obsolete: true
Keywords: checkin-needed
No sr needed for browser so it is ok to checkin. Thanks!
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: 14 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M7
Attached patch fix mingw build failure (obsolete) — Splinter Review
Not sure why my build fails on this, only now, but anyway.
Attachment #283158 - Flags: review?(robert.bugzilla)
Attachment #283158 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 283158 [details] [diff] [review]
fix mingw build failure

Asking approval1.9 to remove this comma.
Attachment #283158 - Flags: approval1.9?
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.