Remove MOZ_UTF16() macro in favor of using u"string" directly

RESOLVED FIXED in Thunderbird 50.0

Status

MailNews Core
Backend
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: ewong, Assigned: Jorg K (GMT+2))

Tracking

unspecified
Thunderbird 50.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
There's quite a few mentions in mailnews/ etc..  

+++ This bug was initially created as a clone of Bug #1277106 +++

We are currently using MOZ_UTF16() macro to workaround the difference between VC compiler (which uses L"") and other compilers (which accepts u""). This macro can be removed once we drop the support of VS2013, because VS2015 supports u"string" directly.
(Assignee)

Comment 1

a year ago
Created attachment 8773459 [details] [diff] [review]
[see comment #8 and comment #10 for patches landed]

This was created with:

find . -name *.cpp -exec sed -i -e 's/MOZ_UTF16(\(\".*\"\))/u\1/g' {} \;

run on /mail and /mailnews.

Try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=05713963be5a
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
(Assignee)

Comment 2

a year ago
Comment on attachment 8773459 [details] [diff] [review]
[see comment #8 and comment #10 for patches landed]

Anyone wants to review this? It's boring. sed will have done the right thing.
Attachment #8773459 - Flags: review?(acelists)
(Assignee)

Comment 3

a year ago
There were two in /suite. I've got them covered ;-)
The parent patch had also changes to NS_LITERAL_STRING, like:

-      Write(NS_LITERAL_STRING("\xFFFC"));
+      Write(NS_LITERAL_STRING(u"\xFFFC"));

Do you intent to also do a patch for that?
(Assignee)

Comment 5

a year ago
Kent, no.

Joshua pointed out that I missed a few lines where there were two strings on a line. Fixed now:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c802c2cc66df
(Assignee)

Comment 6

a year ago
Missed some more.

I got smart and did a local compile on Windows first ;-) Since that worked, here another try:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=14569ad7ebf1
(Assignee)

Comment 7

a year ago
(In reply to Kent James (:rkent) from comment #4)
> The parent patch had also changes to NS_LITERAL_STRING, like:
> -      Write(NS_LITERAL_STRING("\xFFFC"));
> +      Write(NS_LITERAL_STRING(u"\xFFFC"));
> Do you intent to also do a patch for that?

We don't have that anywhere. I only found this in Mailnews:
  int32_t offset = val.Find(NS_LITERAL_STRING("\x0D\x0A"));
    offset = val.Find(NS_LITERAL_STRING("\x0D\x0A"), offset + 2);
./import/text/src/nsTextImport.cpp

So I don't think they need treatment.
(Assignee)

Comment 8

a year ago
Landed:
https://hg.mozilla.org/comm-central/rev/a09c05db1261

The patch is 4422 lines long, so frankly, I don't thing anyone would have had the time or inclination to go through it with a fine comb. Plus, apart from a few manual tweaks, it was all sed.

Let's leave the bug open to see whether there are any build/test failures. Built OK on Windows and Linux.
(Assignee)

Comment 9

a year ago
Comment on attachment 8773459 [details] [diff] [review]
[see comment #8 and comment #10 for patches landed]

The patch does *not* reflect exactly what was pushed!
Attachment #8773459 - Flags: review?(acelists)
(Assignee)

Comment 10

a year ago
Linux went through alright, Windows didn't build for some reason and Mac failed since I had forgotten to fix up .mm files as well. So here's another push:
https://hg.mozilla.org/comm-central/rev/66ffb3c06576
(Assignee)

Comment 11

a year ago
Mac still doesn't compile, looks like more bustage, but in M-C(??):

/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/Promise.cpp:744:31: error: no member named 'PromiseNativeHandler' in namespace 'mozilla::dom::prototypes::id'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseDebugging.cpp:402:27: error: no viable conversion from 'JS::RootedObject' (aka 'Rooted<JSObject *>') to 'mozilla::dom::Promise'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseDebugging.cpp:419:23: error: no viable conversion from 'JS::RootedObject' (aka 'Rooted<JSObject *>') to 'mozilla::dom::Promise'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseNativeHandler.cpp:19:10: error: use of undeclared identifier 'PromiseNativeHandlerBinding'; did you mean 'PromiseNativeHandler'?
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseNativeHandler.cpp:19:39: error: no member named 'Wrap' in 'mozilla::dom::PromiseNativeHandler'
You need to clobber. Had this with bug 911216 part 30 early this week. Bug has been backed out since then and in again yesterday.
(Assignee)

Comment 13

a year ago
Clobber on Treeherder?? I wouldn't know how to do that, although I can see something here:
https://hg.mozilla.org/mozilla-central/rev/efcf012e6cfc
(Assignee)

Updated

a year ago
Attachment #8773459 - Attachment filename: 1288355.patch → [see comment #8 and comment #10 for patches landed]
(Assignee)

Updated

a year ago
Attachment #8773459 - Attachment description: 1288355.patch → [see comment #8 and comment #10 for patches landed]
Attachment #8773459 - Attachment filename: [see comment #8 and comment #10 for patches landed] → 1288355.patch
I think you just push an empty file called CLOBBER.
I clobbered the OS X (and the Win to be sure) machines. I also restarted the OS X builds.
(Assignee)

Comment 16

a year ago
Richard, can you please explain what you did exactly to clobber.

Restarted the OS X went via the Treeherder menu, "Add New Jobs" I suppose.
(Assignee)

Comment 17

a year ago
Richard told me privately, that "clobber" is also available on the Treeherder menu.

Re. comment #4 and comment #7: Reading bug 1277106 comment #5 (quote):
Note that a few NS_LITERAL_STRING() callers must be changed when their string literal parameter actually contains escaped Unicode characters like "\x2026".

I've just looked again:
find . -name *.cpp -exec grep '\"\\x[0-9][0-9][0-9]' {} \; -print

The only thing that turned up was:
      AddText(L"\x2019"); // Unicode right single quotation mark
./import/outlook/src/rtfMailDecoder.cpp

That L could have been replaced by a u, but since this is compiled for Windows only (Outlook import), it doesn't matter.
(Assignee)

Comment 18

a year ago
Despite looking red on Treeherder, Windows did compile:
http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1469165954/comm-central-win32-bm71-build1-build0.txt.gz
Finished compile (results: 0, elapsed: 59 mins, 4 secs) (at 2016-07-22 00:43:56.597159)
Looks like some packaging failed:
Error: c:\builds\moz2_slave\tb-c-cen-w32-00000000000000000\build\objdir-tb\mail\installer\package-manifest:55: Missing file(s): bin/icudt56l.dat
[snip]
mozmake.exe[3]: *** [stage-package] Error 1
mozmake.exe[2]: *** [make-package] Error 2
mozmake.exe[1]: *** [default] Error 2

Looks like bug 1262731 (SM bug 1288533)
(Assignee)

Comment 19

a year ago
Raised bug 1288651.
Local suite build works fine at first glance. Just briefly tested mail / news. Thanks.
(Assignee)

Comment 21

a year ago
Yes, I started up TB with the change yesterday and it seemed to work.

Since Mac compiled not, I guess we're done here.
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
(Assignee)

Comment 22

a year ago
Opps: Since Mac compiled NOW, I guess we're done here.
You need to log in before you can comment on or make changes to this bug.