1582573 - Indentation of XUL strings in custom elements are off

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Paul Morris [:pmorris]]

The change to two space indent with the Prettier formatting in calendar code left the indentation of the XUL strings passed to parseXULToFragment for custom elements in a messy state. Let's clean them up.

Edit: the indentation of XUL strings in non-calendar custom elements is also off, due to wrapping custom elements in curly brackets and Prettier formatting. Lets use this bug for fixing these too.

Comment 1

[Paul Morris [:pmorris]]

Attachment: xul-string-indent-calendar.patch

Re-indents the XUL strings in calendar custom elements.

Comment 2

[Geoff Lankow (:darktrojan)]

Comment on attachment 9094032 [details] [diff] [review]
xul-string-indent-calendar.patch

TBH, you probably could've gotten away with the "white-space-only" rubber-stamp on this. I had a quick scan and checked the stuff that `diff -w` didn't ignore.

Comment 3

[Paul Morris [:pmorris]]

Attachment: [checked in] xul-string-indent-non-calendar-0.patch

Re-indents XUL strings in non-calendar custom elements as needed. I've never used the whitespace-only option, not sure how it works, so flagging mkmelin for review. Feel free to cancel that if you'd rather I do the whitespace-only option.

Comment 4

[Jorg K (CEST = GMT+2)]

Hmm, this should also go into beta and ESR 68, no? Are we done with reformatting one day?

Comment 5

[Jorg K (CEST = GMT+2)]

Also, this clashes with bug 1582796 which is more important.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a55d5e1c611a
Re-indent XUL strings in non-calendar custom elements. r=mkmelin

Comment 7

[Magnus Melin [:mkmelin]]

Calendar patch needs unbitrotting

Comment 8

[Paul Morris [:pmorris]]

Attachment: xul-string-indent-calendar-1.patch

This just rebases the calendar patch.

"This should also go into beta and ESR 68, no?"
Yeah, I suppose so. (I'm still not 100% sure how you indicate that among all the knobs bugzilla provides.) Aren't we about to release another beta? If so, then maybe we don't need to backport to beta. I can upload patches for ESR.

"Are we done with reformatting one day?"
I hope so, although there are other similar places with multi-line template strings that may be off. I've explored them a bit and they do not seem horrible. I could see the case for just fixing them as we are editing the code, rather than going hunting for every instance.

Comment 9

[Paul Morris [:pmorris]]

Comment on attachment 9094610 [details] [diff] [review]
xul-string-indent-calendar-1.patch

Something went wrong with patch export. :-/  New patch in a sec.

Comment 10

[Paul Morris [:pmorris]]

Attachment: xul-string-indent-calendar-1.patch

Fixed.

Comment 11

[Jorg K (CEST = GMT+2)]

Aren't we about to release another beta? If so, then maybe we don't need to backport to beta. I can upload patches for ESR.

Yes, TB 70 beta 2 was released yesterday (Sunday!). Between now and 2019-10-21 (see https://wiki.mozilla.org/Release_Management/Calendar) we'll still be doing a TB 70 beta 3, maybe even 4. So backporting makes sense to avoid merge conflicts later. I tried one of the patches and the already didn't apply to beta. Yes, ESR is also required.

Comment 12

[Paul Morris [:pmorris]]

Attachment: esr68-xul-string-indent-non-calendar-0.patch

1 of 2 for ESR 68.

Comment 13

[Paul Morris [:pmorris]]

Attachment: esr68-xul-string-indent-calendar-0.patch

2 of 2 for ESR 68.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cf7a0bacde48
Re-indent XUL strings in calendar custom elements. r=darktrojan

Comment 15

[Jorg K (CEST = GMT+2)]

Geoff, can you please confirm beta/esr approval for the Calendar bits. I assume you can't set it and I can't request Calendar approval on this bug.

Comment 16

[Paul Morris [:pmorris]]

Attachment: beta-xul-string-indent-non-calendar-0.patch

1 of 2 for beta.

Comment 17

[Paul Morris [:pmorris]]

Attachment: beta-xul-string-indent-calendar-0.patch

2 of 2 for beta.

Comment 18

[Geoff Lankow (:darktrojan)]

Approved.

Comment 19

[Jorg K (CEST = GMT+2)]

Comment on attachment 9094664 [details] [diff] [review]
beta-xul-string-indent-calendar-0.patch

Setting flags according to Geoff's approval.

Comment 20

[Jorg K (CEST = GMT+2)]

TB 68.1.1 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/1696158cf843ece6687d579ce8f4eded49d967ef
https://hg.mozilla.org/releases/comm-esr68/rev/1517231ef9e8bfc085781805e5fa70a65cf99dfd

Comment 21

[Jorg K (CEST = GMT+2)]

TB 70 beta 3:
https://hg.mozilla.org/releases/comm-beta/rev/f05598962defbc669b8fad2436c85bc91a77420c
https://hg.mozilla.org/releases/comm-beta/rev/cb916f5a2e9a5364eff8e99c81b12584274bc009