Closed Bug 1213246 Opened 4 years ago Closed 4 years ago

Fix WinIMEHandler.cpp compilation on compilers with builtin char16_t support.

Categories

(Core :: Widget: Win32, defect)

Unspecified
Windows
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1213123
Tracking Status
firefox44 --- affected

People

(Reporter: jacek, Assigned: jacek)

References

Details

Attachments

(1 file)

Attached patch fixSplinter Review
No description provided.
Attachment #8671841 - Flags: review?(masayuki)
Comment on attachment 8671841 [details] [diff] [review]
fix

>From: Jacek Caban <jacek@codeweavers.com>
>Fixed WinIMEHandler.cpp compilation on compilers with builtin char16_t support.
>
>
>diff --git a/widget/windows/WinIMEHandler.cpp b/widget/windows/WinIMEHandler.cpp
>index f39688f..185be63 100644
>--- a/widget/windows/WinIMEHandler.cpp
>+++ b/widget/windows/WinIMEHandler.cpp
>@@ -586,36 +586,36 @@ IMEHandler::WStringStartsWithCaseInsensitive(const std::wstring& aHaystack,
> // are attached to the machine.
> // Based on IsKeyboardPresentOnSlate() in Chromium's base/win/win_util.cc.
> // static
> bool
> IMEHandler::IsKeyboardPresentOnSlate()
> {
>   // This function is only supported for Windows 8 and up.
>   if (!IsWin8OrLater()) {
>-    Preferences::SetString(kOskDebugReason, L"IKPOS: Requires Win8+.");
>+    Preferences::SetString(kOskDebugReason, MOZ_UTF16("IKPOS: Requires Win8+."));

Over 80 chars, please change this as:

    Preferences::SetString(kOskDebugReason,
                           MOZ_UTF16("IKPOS: Requires Win8+."));

>     return true;
>   }
> 
>   if (!Preferences::GetBool(kOskDetectPhysicalKeyboard, true)) {
>-    Preferences::SetString(kOskDebugReason, L"IKPOS: Detection disabled.");
>+    Preferences::SetString(kOskDebugReason, MOZ_UTF16("IKPOS: Detection disabled."));

Same.

>@@ -659,34 +659,34 @@ IMEHandler::IsKeyboardPresentOnSlate()
>   PowerDeterminePlatformRole power_determine_platform_role =
>     reinterpret_cast<PowerDeterminePlatformRole>(::GetProcAddress(
>       ::LoadLibraryW(L"PowrProf.dll"), "PowerDeterminePlatformRole"));
>   if (power_determine_platform_role) {
>     POWER_PLATFORM_ROLE role = power_determine_platform_role();
>     if (((role == PlatformRoleMobile) || (role == PlatformRoleSlate)) &&
>          (::GetSystemMetrics(SM_CONVERTIBLESLATEMODE) == 0)) {
>       if (role == PlatformRoleMobile) {
>-        Preferences::SetString(kOskDebugReason, L"IKPOS: PlatformRoleMobile.");
>+        Preferences::SetString(kOskDebugReason, MOZ_UTF16("IKPOS: PlatformRoleMobile."));

Same.

>       } else if (role == PlatformRoleSlate) {
>-        Preferences::SetString(kOskDebugReason, L"IKPOS: PlatformRoleSlate.");
>+        Preferences::SetString(kOskDebugReason, MOZ_UTF16("IKPOS: PlatformRoleSlate."));

Same.

>   // Query for all the keyboard devices.
>   HDEVINFO device_info =
>     ::SetupDiGetClassDevs(&KEYBOARD_CLASS_GUID, nullptr,
>                           nullptr, DIGCF_PRESENT);
>   if (device_info == INVALID_HANDLE_VALUE) {
>-    Preferences::SetString(kOskDebugReason, L"IKPOS: No keyboard info.");
>+    Preferences::SetString(kOskDebugReason, MOZ_UTF16("IKPOS: No keyboard info."));

Same.


>@@ -708,83 +708,83 @@ IMEHandler::IsKeyboardPresentOnSlate()
>                                                        L"ACPI") ||
>           IMEHandler::WStringStartsWithCaseInsensitive(device_id,
>                                                        L"HID\\VID")) {
>         // The heuristic we are using is to check the count of keyboards and
>         // return true if the API's report one or more keyboards. Please note
>         // that this will break for non keyboard devices which expose a
>         // keyboard PDO.
>         Preferences::SetString(kOskDebugReason,
>-                               L"IKPOS: Keyboard presence confirmed.");
>+                               MOZ_UTF16("IKPOS: Keyboard presence confirmed."));

Hmm, please use this form:

        Preferences::SetString(kOskDebugReason,
          MOZ_UTF16("IKPOS: Keyboard presence confirmed."));

>   bool isInTabletMode = false;
>   uiUtils->GetInTabletMode(&isInTabletMode);
>   if (isInTabletMode) {
>-    Preferences::SetString(kOskDebugReason, L"IITM: GetInTabletMode=true.");
>+    Preferences::SetString(kOskDebugReason, MOZ_UTF16("IITM: GetInTabletMode=true."));

    Preferences::SetString(kOskDebugReason,
                           MOZ_UTF16("IITM: GetInTabletMode=true."));

>   } else {
>-    Preferences::SetString(kOskDebugReason, L"IITM: GetInTabletMode=false.");
>+    Preferences::SetString(kOskDebugReason, MOZ_UTF16("IITM: GetInTabletMode=false."));

    Preferences::SetString(kOskDebugReason,
                           MOZ_UTF16("IITM: GetInTabletMode=false."));

> // static
> bool
> IMEHandler::AutoInvokeOnScreenKeyboardInDesktopMode()
> {
>   nsresult rv;
>   nsCOMPtr<nsIWindowsRegKey> regKey
>     (do_CreateInstance("@mozilla.org/windows-registry-key;1", &rv));
>   if (NS_WARN_IF(NS_FAILED(rv))) {
>-    Preferences::SetString(kOskDebugReason, L"AIOSKIDM: "
>-                           L"nsIWindowsRegKey not available");
>+    Preferences::SetString(kOskDebugReason,
>+                           MOZ_UTF16("AIOSKIDM: nsIWindowsRegKey not available"));

    Preferences::SetString(kOskDebugReason,
      MOZ_UTF16("AIOSKIDM: nsIWindowsRegKey not available"));

>   // EnableDesktopModeAutoInvoke is an opt-in option from the Windows
>   // Settings to "Automatically show the touch keyboard in windowed apps
>   // when there's no keyboard attached to your device." If the user has
>   // opted-in to this behavior, the tablet-mode requirement is skipped.
>   uint32_t value;
>   rv = regKey->ReadIntValue(NS_LITERAL_STRING("EnableDesktopModeAutoInvoke"),
>                             &value);
>   if (NS_FAILED(rv)) {
>     Preferences::SetString(kOskDebugReason,
>-                           L"AIOSKIDM: failed reading value of regkey.");
>+                           MOZ_UTF16("AIOSKIDM: failed reading value of regkey."));

    Preferences::SetString(kOskDebugReason,
      MOZ_UTF16("AIOSKIDM: failed reading value of regkey."));

>     return false;
>   }
>   if (!!value) {
>-    Preferences::SetString(kOskDebugReason, L"AIOSKIDM: regkey value=true.");
>+    Preferences::SetString(kOskDebugReason, MOZ_UTF16("AIOSKIDM: regkey value=true."));

    Preferences::SetString(kOskDebugReason,
                           MOZ_UTF16("AIOSKIDM: regkey value=true."));

>   } else {
>-    Preferences::SetString(kOskDebugReason, L"AIOSKIDM: regkey value=false.");
>+    Preferences::SetString(kOskDebugReason, MOZ_UTF16("AIOSKIDM: regkey value=false."));

    Preferences::SetString(kOskDebugReason,
                           MOZ_UTF16("AIOSKIDM: regkey value=false."));


Please keep 80 chars per line rule.

I'd like to check new patch, and I'll give r+ for the new patch with those changes. Then, if you'd like me to land the patch at r+'ing, let me know.
Attachment #8671841 - Flags: review?(masayuki) → review-
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1213123
You need to log in before you can comment on or make changes to this bug.