1509102 - Serialising a tag attribute with a newline in the string attribute value results in 
 resulting in broken roundtrip - STR: comment #3

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[Heine Svendsen]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

Every time I edit a message these characters  &#xA  are added to for instance <td style>
This has started with the latest version Thunderbird 60.3.1 32-bit.
And only when ThunderHTMLedit is installed and active.
Just opening an email for editing and then closing it again (with no edits), makes Thunderbird ask me, if I want to save the changes to the email.
For every save, ThunderHTMLedit (or Thunderbiord?) doubles the instances of "&#xA" added, so it quickly becomes a bit of a problem, even though these extra characters does not affect the apperance of the email.


Actual results:

This:
    <table style="font-size: 12px; font-family: Verdana,
      Arial, Helvetica; box-shadow: #999999 0px 0px 10px 0px;
      border: 0px; solid; #a0a0a0;" max-width="600"
      cellspacing="0" cellpadding="0" border="0" align="center">

Quickly becomes like this:
    <table style="font-size: 12px; font-family: Verdana,&#xA;&#xA;
      Arial,&#xA;&#xA; Helvetica; box-shadow: #999999 0px 0px 10px&#xA;
      0px;&#xA; border: 0px&#xA; solid&#xA; #a0a0a0;" max-width="600"
      cellspacing="0" cellpadding="0" border="0" align="center">
(notise the   &#xA;



Expected results:

Nothing should have happened, since I did not change anything

Comment 1

[Jorg K (CEST = GMT+2)]

Add-on problems should not be reported here, they should be reported to the add-on author.

Coincidentally, I am the author of that add-on, and I had the same complaint in a PM from someone else. My reply was:

  The add-on only replaces \xA0 with   and tabs with 	
  so you can actually see that stuff. The replacements you mention
  must be from the Mozilla platform serialisation of the DOM tree to text.

That said, since this is the second complaint, so Masayuki-san, has serialising changed recently so &#A; are now produced?

The other reporter said that this
  <div class="signature" style="margin: 5px auto;  <--------- line break here
  padding:10px;">Hello</div>

is turned into this
  <div class="signature" style="margin: 5px auto;&#xA;
  padding:10px;">Hello </div> 

when this is in a signature.

Reporter: Do you have that table in your signature?

Comment 2

[Heine Svendsen]

Hi Jorg K

Thanks for answering. I tried to find you, so I could ask you directly, but couldn't.
Since I wrote about this problem, I found out, that your add-on is not (directly) to blame (and maybe not at all).

The problems is the same, when you open an email to edit it (ThunderHTMLedit not installed), and you then choose to save it (no changes made).
I only noticed the extra code, because you need ThunderHTMLedit to see it, or you can also see it with the meny Insert > Html...

It's just opening an email for editing, that's the cause of the extra codes added.

But if you keep the email open for editing, and goes into the build-in HTML-editor (insert > HTML...), the a certain number of this code "&#xA" are added every time you open the editor and close it again.

There is a difference, though.
If ThunderHTMLedit is not active, I can open emails for editing and close them again directly with no "questions asked".
If ThunderHTMLedit is active just opening an email for editing, but making no changes and not going into the editor, I will then be prompted if I want the save the changes, upon closing the email.

Did you just update your add-on????
An hour ago, if I opened a draft and saved it, the draft would be doubled.
If I saved it again, I would have three of the same drafts.
This never happened with TB 59. And now it doesn't with TB 60 either it seems????
And I think, when disabling your add-on, it didn't just say "restart to disable", but instead it said "Restart to installe update".


This is something I posted on support.mozilla.org yesterday:

1. The problem is with TB 60.x.x, not with 52.9.1, that I just installed.
Or maybe TB 52 just doesn't show these linefeed commands & # X a ;

2. TB 60 has this problem only with emails that includes tables.
I also get these extra linefeed characters in many commercial emails, that I have received from others, but only if they have tables. Not all emails with tables have the problem, so some must comply better to the "TB-standard" than others.

3. just opening one of my own emails with tables and closing it again prompts for Save or Cancel.
If one of these emails have been sent to someone, who answers me back, with the original email included, then I can open the email and close it without being prompted to save it.
The email still contains the line feed codes though, and if I actually save the mail, extra line feed codes will beadded.

3. If an email has - says 20 of these erroneous codes for line feed, then it will have 40 after saving it. Opening it and saving it again, it will end up with 60 (not 80, so not double).

4. TB 52.9.1 doesn't show any line feed codes, not even in mails, that had hundreds of them when examined in TB 60.
Either TB 52 cannot show them, or they get filtered out.
I opened an email that had these line feeds, when I looked at it in the TB 60 editor, and there were no line feed codes in the editor of TB 52. When I saved the email from TB 52, it shrunk from 17 kb to 10 kb. The difference is much larger than the number of line feeds could account for, so maybe this is not significant.


It boils down to this: the tags should be made in TB with it's own formatting tools. I used my own styles to get around design problems, and TB does not cope well with this. I put style codes inside <table> tags, but I should instead put them inside <td> tags, for sure.

PS: a really annoying thing: drafts, that were edited with TB 60, and are now edited with TB 52, will produce en new draft every time it is saved instead of overwriting the old draft. It's like saving templates, where you have always had a new file for each save.

Comment 3

[Jorg K (CEST = GMT+2)]

OK, most likely we're looking at a Core::Editor bug here that the Mozilla team will need to fix. It's not limited to tables, it happens for all tag attribute strings broken across lines.

So please:
- Let's not talk about my add-on (yes, it was updated)
- Let's not talk about any other Thunderbird issues.
I'm obsoleting all comments which distract from the real problem.

We have a very valuable contributor who can find where a problem started occurring, so for him we need very simple steps to reproduce the problem.

So here goes. STR:

Create a new HTML message. Click into the body. Form the menu: Insert > HTML. Insert this:
<div style="font-size: 12px; font-family: Verdana,
  Arial, Helvetica;">
</div>
Save the message as draft, inspect the message source.

Bad:
<div style="font-size: 12px; font-family: Verdana,&#xA; Arial,
   Helvetica;">
</div>
Notice the unwanted &#xA;

Alice, when did this start? TB 52 doesn't do that. Can you find the regression range for us.

Comment 5

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=08ae795fa05906491189f056ea3c4d8fca0dfe83&tochange=8ebb03cbea92cf0f8d60745ec5b3b99140eacf07
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0d806b3230fe4767fa70cdee57db87d1e9a5ba49&tochange=f1a4b64f19b0e93c49492735db30a5023e624ae7

Comment 6

[Jorg K (CEST = GMT+2)]

Thanks Alice, that looks like this changeset caused it:
fe4514c84e7a Ashish Kulkarni - Bug 169521: fix XML attribute serialization for proper roundtripping r=bz
https://hg.mozilla.org/mozilla-central/rev/fe4514c84e7a#l1.32

Boris and Ashish: Looks like serialising now returns 
 which causes strive in TB. As you can see in comment #3, the round trip doesn't actually work, since the line is now broken after the
  style="font-size: 12px; font-family: Verdana,
 Arial,
which will lead to another 
 being inserted.

Comment 7

[Jorg K (CEST = GMT+2)]

For e-mail the roundtrip is this:

Start with a text file (the e-mail) and parse this into a DOM. Display that DOM in an editor. When editing is done, serialise DOM back to text file. The latter seems to add the 
 now, but the former doesn't remove them.

But that's not all. For "sanitising" HTML e-mail, we actually add another full round trip here:
https://dxr.mozilla.org/comm-central/rev/2a29ee0adb310b54a6a2df72034953fed8f2b043/comm/mailnews/mime/src/mimeTextHTMLParsed.cpp#91

That code was only added in May 2018 here
https://hg.mozilla.org/comm-central/rev/96fab4a2b811
but it aggravates the problem that started in February 2018.

Adding some debug I see:

Read from the draft/file:
    <div style="font-size: 12px; font-family: Verdana,&#xA; Arial,
      Helvetica;">
(where the &#xA; already resulted from a previous serialisation of the DOM when writing the draft).

After the roundtrip in MimeInlineTextHTMLParsed_parse_eof(), that is, ParseFromString() and EncodeToString():
    <div style="font-size: 12px; font-family: Verdana,&#xA; Arial,&#xA;      Helvetica;">
(note the six spaces before Helvetica are still there).

So I guess there is a bug in the DOMParser which should have undone the inserted &#xA;

NI'ing Henri for the DOM parser here.

Comment 8

[Jorg K (CEST = GMT+2)]

Looking at this again, I don't think we want to touch the DOM parser here. If bug 169521 was about serialising XML, why is HTML affected? I guess that's the real bug here. Moving this to Core::Serializers is of course dangerous since that's un-owned :-(

The quick fix for TB will be a backout of bug 169521 from TB's branch on mozilla-esr60.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> If bug 169521 was about serialising XML, why is HTML affected?

nsHTMLContentSerializer inherits from nsXHTMLContentSerializer which inherits from nsXMLContentSerializer.  nsHTMLContentSerializer::SerializeHTMLAttributes calls nsXMLContentSerializer::SerializeAttr which does the serialization.  Shouldn't be a problem per se, in general.

> So I guess there is a bug in the DOMParser which should have undone the inserted 


Testing in the Firefox console, 

  (new DOMParser()).parseFromString("<div foo='a&#xA;b'>", "text/html").querySelector("div").getAttribute("foo")

shows a string with a newline in it, which means the &#xA; got parsed into a newline, as expected.  Why exactly do you think there's an issue with DOMParser here?

As far as I can tell, all the core bits are working fine.  The serializer outputs an entity reference.  The parser converts it to a newline character.  This is all how it should work,.

It's true that if you start with raw newlines and then go through this process they will get converted to &#xA;.  The DOM representation has no way to tell whether the original source had raw newlines or &#xA;, so you will get roundtrip failures no matter what you do here...  I suppose we could restrict the behavior from bug 169521 to XML, in which case HTML with &#xA; on disk will fail to round-trip.

None of that accounts for why the number of newlines keeps increasing, as reported above.  That presumably depends on the exact serialization flags Thunderbird is using.  When you land in nsXMLContentSerializer::SerializeAttr for the relevant attribute, which of the codepaths do you end up following in Thunderbird?  Is it AppendToStringConvertLF or AppendToStringFormatedWrapped or AppendToStringWrapped?  I am guessing that what happens is that the newline is converted to &#xA, but then Thunderbird's serialization adds an extra newline anyway to wrap things nicely or some such.  The next time it's parsed (as HTML) the &#xA; becomes a newline, and the newline that was added is still there, so you get two newlines, etc.

Comment 10

[Jorg K (CEST = GMT+2)]

Hmm, as I said in comment #7, the problem is already visible in about 15 lines of code here:
https://dxr.mozilla.org/comm-central/rev/2a29ee0adb310b54a6a2df72034953fed8f2b043/comm/mailnews/mime/src/mimeTextHTMLParsed.cpp#91-107

Original 'rawHTML' is
    <div style="font-size: 12px; font-family: Verdana,&#xA; Arial,
      Helvetica;">
(where the &#xA; already resulted from a previous serialisation of the DOM when writing the draft).

'parsed' comes back as:
<div style="font-size: 12px; font-family: Verdana,&#xA; Arial,&#xA;      Helvetica;">

So it looks like the serialisation added the &#xA; after "Arial" when it didn't before and the parser didn't remove it.

I'll answer your questions in a minute.

Comment 11

[Jorg K (CEST = GMT+2)]

OK, when saving the draft, so NOT the code quote above, we come through here:
  if (mDoRaw || PreLevel() > 0) {
    NS_ENSURE_TRUE(AppendToStringConvertLF(attrString, aStr), false);
    printf("=== (1)\n");
  }
  else if (mDoFormat) {
    NS_ENSURE_TRUE(AppendToStringFormatedWrapped(attrString, aStr), false);
    printf("=== (2)\n");  <======
  }
  else if (mDoWrap) {
    NS_ENSURE_TRUE(AppendToStringWrapped(attrString, aStr), false);
    printf("=== (3)\n");
  }
  else {
    NS_ENSURE_TRUE(AppendToStringConvertLF(attrString, aStr), false);
    printf("=== (4)\n");
  }

Saving the draft is most likely
https://searchfox.org/comm-central/rev/aa357565819d38429d69190fbcc80f149c6488fe/mailnews/compose/src/nsMsgSend.cpp#1493
which uses nsIDocumentEncoder::OutputFormatted. There the &#xA; is already undesired.

Displaying the message, so the MIME code I quoted, doesn't appear to trigger that block.

Comment 12

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #11)
> Displaying the message, so the MIME code I quoted, doesn't appear to trigger
> that block.
Correct, it instead triggers this block just above:
  if (aDoEscapeEntities) {
    printf("=== (0)\n");  <====

I hope comment #11 was clear enough, the code path printing "=== (2)\n" was executed. All that changed somehow :-(

Comment 13

[Jorg K (CEST = GMT+2)]

If I revert rev fe4514c84e7a everything becomes good again. The saved draft doesn't contain the 
 and the code mimeTextHTMLParsed.cpp#91-107 shows:
    <div style="font-size: 12px; font-family: Verdana, Arial,
      Helvetica;">
and then unchanged:
    <div style="font-size: 12px; font-family: Verdana, Arial,
      Helvetica;">

Comment 14

[Jorg K (CEST = GMT+2)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #9)
> None of that accounts for why the number of newlines keeps increasing, as
> reported above.
Actually, it does. If saving a draft adds a 
 and then editing the draft breaks the line elsewhere, then anther one is added.

Does that give enough information to work with? I think I'll just backout rev fe4514c84e7a for TB 60.

Comment 15

[Boris Zbarsky [:bzbarsky]]

> So it looks like the serialisation added the 
 after "Arial" when it didn't before

More precisely, it replaced the \n with &xA;.  But it might have added _another_ \n.

> the code path printing "=== (2)\n" was executed

This is the part I was looking for.  That's definitely going to add newlines.

It looks like we have two different kAttrEntities bits, one in the HTML serializer one in the XML one.  The HTML one is only used for the nsIDocumentEncoder::OutputEncodeBasicEntities case, and encodes quot/amp/lt/gt/nbsp.  The XML one used to only do the first 4 of those until the changes in bug 169521.

The right fix here is probably to introduce an nsXMLContentSerializer method that represents most of the work of  nsXMLContentSerializer::AppendAndTranslateEntities but takes an entity table argument (or probably an entity table, a string table, and a max value).  Then we could call it from the HTML serializer with the HTML entity tables.

Writing that fix is easy; the hard part is writing the tests.  What cases does Thunderbird actually care about here?  Can you create automated tests for them (which presumably fail on trunk right now)?

Comment 16

[Jorg K (CEST = GMT+2)]

Reading my comments above, they are a bit convoluted, so I'll restate them here:

Displaying a message runs
https://dxr.mozilla.org/comm-central/rev/2a29ee0adb310b54a6a2df72034953fed8f2b043/comm/mailnews/mime/src/mimeTextHTMLParsed.cpp#91-107
and this
  if (aDoEscapeEntities) {
    printf("=== (0)\n");  <====
printing "=== (0)\n".

Saving a draft (most likely) runs this
https://searchfox.org/comm-central/rev/aa357565819d38429d69190fbcc80f149c6488fe/mailnews/compose/src/nsMsgSend.cpp#1493
and prints "=== (2)\n". You confirmed that that adds "newlines", umm, you mean those &#xA;?

I'm also interested in round trip in mimeTextHTMLParsed.cpp#91-107.
    <div style="font-size: 12px; font-family: Verdana, Arial,
      Helvetica;">
gets turned into
    <div style="font-size: 12px; font-family: Verdana, Arial,&#xA;      Helvetica;">
but maybe that doesn't matter. I noticed that running our "Edit Draft" command doesn't trigger any debug output, so loading the draft again doesn't seem to add another &#xA;. If it did, saving and edit/save cycle would add two &#xA;, but only one gets added when saving.

As for writing tests: There are pure C++/gTests in M-C, so one of those could be added. We can of course also write tests to check message content during a save/edit again cycle.

What we care about is that no additional &#xA; get saved to disk, my add-on ThunderHTMLedit which started this bug, also uses the editor's outputToString() as the send does, so that also goes through (2) and shows undesired &#xA;

Personally I'd also like to understand what has changed on the (0) code path.

Comment 17

[Boris Zbarsky [:bzbarsky]]

> umm, you mean those 
?

No, I mean actual newlines.

> gets turned into

Right, that part is expected given the current setup.  Whether it's desirable, I don't know.

> There are pure C++/gTests in M-C, so one of those could be added.

Yes, I'm aware of what the general idea is.  I just don't have time myself to figure out what behaviors Thunderbird depends on here and write tests for them.

> What we care about is that no additional 
 get saved to disk

But you don't care if 
 that are actually in the files get lost, right?

> Personally I'd also like to understand what has changed on the (0) code path.

You mean other than bug 169521?

Comment 18

[Jorg K (CEST = GMT+2)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #17)
> > What we care about is that no additional 
 get saved to disk
> But you don't care if 
 that are actually in the files get lost, right?
Well, ideally a roundtrip compose/save draft/edit draft would leave us where the started as it has always been. I can write a test for that in an hour (or less).

> > Personally I'd also like to understand what has changed on the (0) code path.
> You mean other than bug 169521?
No, I meant from that bug. Before you stated that the (2) code path (save draft) would add newlines (which later are turned into 
?). 

Right now you commented on the (0) path, the mimeTextHTMLParsed.cpp#91-107 used to display a message:
> > gets turned into
> Right, that part is expected given the current setup.  Whether it's desirable, I don't know.

so it looks like you figured out where the extra 
 comes from for both, right?

Comment 19

[Boris Zbarsky [:bzbarsky]]

> would leave us where the started as it has always been.

It hasn't been.  The pre-60 behavior was that 
 in the source file would get converted to \n on save.

> No, I meant from that bug.

What changed is that \n gets converted to 
.

> so it looks like you figured out where the extra 
 comes from for both, right?

Look, you start with a \n.  You parse it, you now have a \n in the DOM.  You go to serialize, convert the \n to 
.  The serializer then tries to output that string, sees that it's going to be a really long line, and inserts a newline (\n).  This step used to not happen because there was already a \n there, but now that's been converted to 
.  So you started with "\n" and by the time you save you are saving "
\n".

Then you parse it again, the DOM now has "\n\n".  You serialize, you have "

" and the serializer helpfully breaks your long line so now you save "

\n".  And so forth.

Comment 20

[Jorg K (CEST = GMT+2)]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #19)
> It hasn't been.  The pre-60 behavior was that 
 in the source file would
> get converted to \n on save.
Alright. Well, no one complained about that. In fact, bug 1508591 comment #0, appear to suggest that TB used to remove "bad" 
 when it found them.

> What changed is that \n gets converted to 
.
OK, that the mimeTextHTMLParsed.cpp#91-107 bit:
Feed this
    <div style="font-size: 12px; font-family: Verdana, Arial,\n      Helvetica;">
to the parser and get this when serialising it "unformatted" again:
    <div style="font-size: 12px; font-family: Verdana, Arial,&#xA;      Helvetica;">
Here is doesn't matter since the longer line will eventually be parsed again and then the \n goes back into the DOM.

> Look, you start with a \n.  You parse it, you now have a \n in the DOM.  You
> go to serialize, convert the \n to &#xA;.  The serializer then tries to
> output that string, sees that it's going to be a really long line, and
> inserts a newline (\n).  This step used to not happen because there was
> already a \n there, but now that's been converted to &#xA;.  So you started
> with "\n" and by the time you save you are saving "&#xA;\n".

I guess, finally the penny dropped ;-) - As you've already stated in comment #9, the parsing bit is OK, in the DOM there are always \n.

So if we can get a fix here, I can write a test.

Note the following unrelated item: You kindly provided this API
https://dxr.mozilla.org/comm-central/rev/2a29ee0adb310b54a6a2df72034953fed8f2b043/comm/mailnews/mime/src/mimeTextHTMLParsed.cpp#94
once upon a time for use in
https://searchfox.org/comm-central/rev/efacd58fa63199a34709dc95b05bb9ab56d45f13/mailnews/import/winlivemail/nsWMUtils.cpp#140
and that has since found use on the main code path for message display. Sadly we never wrote a test for it, but I guess TB would fall apart if that stopped working.

Comment 21

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/9f4280a52132bd8335d4833304e4bb7ee4d7c07d
Backed out part of changeset fe4514c84e7a (bug 169521 for causing bug 1509102) to build Thunderbird 60.3.2. a=jorgk

Comment 22

[Ashish Kulkarni [:ashkulz]]

(In reply to Jorg K (GMT+1) from comment #20)
> I guess, finally the penny dropped ;-) - As you've already stated in comment
> #9, the parsing bit is OK, in the DOM there are always \n.
> 
> So if we can get a fix here, I can write a test.

I'm willing to work on a fix if you add a test :-)

Comment 23

[Boris Zbarsky [:bzbarsky]]

Attachment: Stop converting \n to 
 in attribute values when serializing HTML

Comment 24

[Boris Zbarsky [:bzbarsky]]

Jorg, does that patch work?

Ashish, sorry, I had that written before I saw comment 22....

Comment 25

[Ashish Kulkarni [:ashkulz]]

No worries, great that you've already done it!

Comment 26

[Jorg K (CEST = GMT+2)]

Comment on attachment 9027792 [details] [diff] [review]
Stop converting \n to 
 in attribute values when serializing HTML

Review of attachment 9027792 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Boris, yes, the patch works fine.

::: dom/base/nsHTMLContentSerializer.cpp
@@ +472,5 @@
>    }
>  
> +  // We don't want to call into our superclass 2-arg version of
> +  // AppendAndTranslateEntities, because it wants to encode more characters
> +  // than we do.  Use our tables, but avoir encoding   by passing in a smaller max index.

typo: avoid.

::: dom/base/nsXMLContentSerializer.h
@@ +191,5 @@
> +   *
> +   * The first integer template argument, which callers need to specify
> +   * explicitly, is the index of the last entry in aEntityTable that should be
> +   * considered for encoding as an entity reference.  The second integer
> +   * argument will be deduced rom the actual table passed in.

typo: from.

Comment 27

[Boris Zbarsky [:bzbarsky]]

Attachment: Stop converting \n to 
 in attribute values when serializing HTML

Comment 28

[(no longer active)]

Comment on attachment 9028005 [details] [diff] [review]
Stop converting \n to 
 in attribute values when serializing HTML

Review of attachment 9028005 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsHTMLContentSerializer.cpp
@@ +473,5 @@
>  
> +  // We don't want to call into our superclass 2-arg version of
> +  // AppendAndTranslateEntities, because it wants to encode more characters
> +  // than we do.  Use our tables, but avoid encoding   by passing in a
> +  // smaller max index.

This comment is good, but I think it would be a bit better to clarify further what characters would be encoded in the two main branches in this function (the OutputEncodeBasicEntities case vs this one.)  I'd appreciate if you could add a comment to help clarify that a bit.

::: dom/base/nsXMLContentSerializer.h
@@ +214,5 @@
> +
> +  /**
> +   * Max index that can be used with some of our entity tables.
> +   */
> +  static const uint16_t kGTVal = 62;

Now that you're defining this, https://searchfox.org/mozilla-central/rev/d35199ef15ffa57d190886e20d5df6471faf0870/dom/base/nsXMLContentSerializer.cpp#1178 is unneeded, right?  Can you please get rid of it here as well?

Comment 29

[Boris Zbarsky [:bzbarsky]]

> I'd appreciate if you could add a comment to help clarify that a bit.

Done.

> Can you please get rid of it here as well?

The patch does that, yes...

Comment 30

[Boris Zbarsky [:bzbarsky]]

Jorg, I would really like to have some tests here before checking this in...  Do you think you can do that?

Comment 31

[Jorg K (CEST = GMT+2)]

Hmm, I was thinking more of tests in TB for the round trip. I could try some tests for M-C too, but in a follow-up. Or do do you want to wait? I'm pretty busy (still) chasing regressions in TB 60.x, this is just one of them.

Comment 32

[Boris Zbarsky [:bzbarsky]]

I'd like to have m-c tests, since this is all inside m-c serializers.  

I can wait, especially because I want to make sure that the tests fail without the patch and pass with before I check in.  Do you have an approximate timeframe?

Comment 33

[Jorg K (CEST = GMT+2)]

Hmm, a few days, maybe until the weekend. The other bug landed this:
https://hg.mozilla.org/mozilla-central/rev/fe4514c84e7a#l2.13

Would it be much more than
let input = "<div foo='a&#xA;b'>";
let doc  = (new DOMParser()).parseFromString(input, "text/html");
let encoder = Cu.createDocumentEncoder("text/html");
encoder.init(doc, "text/html", 0);
let output = encoder.encodeToString();

Then some check that input.replace("&#xA;", "\n") is pretty much like output?

What steps do you have in mind? And where to best put such a test?

Comment 34

[Boris Zbarsky [:bzbarsky]]

Attachment: With tests

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fc810b6e528
Stop converting \n to 
 in attribute values when serializing HTML.  r=ehsan

Comment 36

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e8744eac2ca
Backed out changeset 8fc810b6e528 for xpcshell failures from test_serializers_entities.js. CLOSED TREE

Comment 37

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 8fc810b6e528 (Bug 1509102) for xpcshell failures from test_serializers_entities.js and mochitest failures on /test_browserElement_oop_TargetBlank.html.

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/7e8744eac2cae3d8d944a3c103cb63b104fb27ed

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=8fc810b6e52834f8e87aa1118b56ade43e26d7ba&selectedJob=214333122

Failure log: 
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=214333122&repo=mozilla-inbound&lineNumber=10345
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=214332872&repo=mozilla-inbound&lineNumber=5072

Comment 38

[Boris Zbarsky [:bzbarsky]]

Ugh, Windows newlines different from everyone else's newlines...  I guess now I get to do some Windows try runs.

Comment 39

[Jorg K (CEST = GMT+2)]

I can take care of it.

Comment 40

[Boris Zbarsky [:bzbarsky]]

I already have a fix running on try, fwiw.

Comment 41

[Jorg K (CEST = GMT+2)]

Ah this one, https://treeherder.mozilla.org/#/jobs?repo=try&revision=0de36b5eff20227629f24a7cc76a0fa6f02f3c0c.
I'll check it locally when my build is done.

Comment 42

[Jorg K (CEST = GMT+2)]

Both pass here (on Windows), thanks again!

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e3b1b073d5e
Stop converting \n to 
 in attribute values when serializing HTML.  r=ehsan

Comment 44

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/9e3b1b073d5e

Comment 45

[Christophe]

Resolved issue for us after the update: https://www.thunderbird.net/en-US/thunderbird/60.3.2/releasenotes/

My initial bug: 1508591

Thank you