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

RESOLVED FIXED in Thunderbird 50.0

Status

MailNews Core
Backend
RESOLVED FIXED
11 months ago
11 months 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

11 months 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

11 months 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

11 months 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

11 months ago
There were two in /suite. I've got them covered ;-)

Comment 4

11 months ago
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

11 months 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

11 months 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

11 months 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

11 months 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

11 months 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

11 months 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

11 months 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

11 months 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

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

Updated

11 months 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

11 months 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

11 months 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

11 months 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

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

Comment 21

11 months 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: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
(Assignee)

Comment 22

11 months 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.