nsIHTMLEditor::IsVisBreak should be a static method on nsContentUtils. Currently a lot of the stuff which happens under it depend on the HTML editor, but I believe that there is nothing inherently depending on it. I'm filing this as a followup to bug 551704, once we have this method on nsContentUtils, we should start using it in nsHTMLContentSerializer::AppendElementStart.

Comment on attachment 8553251 [details] [diff] [review] Don't depend on the editor in nsDocumentEncoder Oh, right, we don't coalesce adjacent <br>. Yeah, this makes sense.

I just realized that this check doesn't work for this edge case: <div><br></div> I'm submitting a second patch to fix that part, but I already landed the first patch on inbound.

The test for bug 1119503 examines this edge case.

Comment on attachment 8553480 [details] [diff] [review] Part 2: Handle the edge case where the BR element is the only child of a block element r=me

Comment on attachment 8553480 [details] [diff] [review] Part 2: Handle the edge case where the BR element is the only child of a block element https://hg.mozilla.org/integration/mozilla-inbound/rev/4ac041f32eee

And we need yet another special case for cases such as <div><br><br></div>. Basically, if the prev sibling is a br, we can ignore it...

The test for bug 1119503 examines this edge case.

https://hg.mozilla.org/mozilla-central/rev/7cea69231f77 https://hg.mozilla.org/mozilla-central/rev/4ac041f32eee

Comment on attachment 8553770 [details] [diff] [review] Part 3: Handle the edge case where the br element appears immediately after another br r=me, though this is getting complicated. Can't we just clam that nonzero height == visible?

(In reply to Boris Zbarsky [:bz] from comment #11) > r=me, though this is getting complicated. Can't we just clam that nonzero > height == visible? Unfortunately not, the BR frame in this case <div>a<br>b</div> for example has both height and width set to 0. It seems like it only gets a non-zero height when it is the only frame on a line, presumably in order to reserve enough vertical space for the line...

Comment on attachment 8553251 [details] [diff] [review] Don't depend on the editor in nsDocumentEncoder Approval Request Comment [Feature/regressing bug #]: related to bug 1119503 [User impact if declined]: none for this particular patch AFAICT but it's part of the set of patches for bug 1119503 which regresses pasting pre-formatted text [Describe test coverage new/current, TreeHerder]: just TreeHerder and nightly since the landing [Risks and why]: elsewhere as part of this set, Ehsan said: "The risk on this patch set is moderate. We have tried to not change the existing behavior except for only the well known cases, but this code is really old and not very well understood. I (ehsan) would like these fixes to be backported on Aurora as soon as we can to get broader testing, and will be on the hook for fixing any regressions found." [String/UUID change made/needed]: UUID change in nsIHTMLEditor (removing BreakIsVisible method)

Comment on attachment 8553480 [details] [diff] [review] Part 2: Handle the edge case where the BR element is the only child of a block element Approval Request Comment [Feature/regressing bug #]: bug 1119503 [User impact if declined]: edge case from bug 1119503 will be handled and that bug deals with copy/pasting pre-formatted text [Describe test coverage new/current, TreeHerder]: try and the time this has been on central [Risks and why]: this particular patch follows up another one on this bug and handles the case where a <br> is the only child of a block element. Elsewhere, Ehsan said: "The risk on this patch set is moderate. We have tried to not change the existing behavior except for only the well known cases, but this code is really old and not very well understood. I (ehsan) would like these fixes to be backported on Aurora as soon as we can to get broader testing, and will be on the hook for fixing any regressions found." [String/UUID change made/needed]: none

Comment on attachment 8553770 [details] [diff] [review] Part 3: Handle the edge case where the br element appears immediately after another br Approval Request Comment [Feature/regressing bug #]: bug 1119503 [User impact if declined]: another edge case from bug 1119503 will be handled and that bug deals with copy/pasting pre-formatted text [Describe test coverage new/current, TreeHerder]: try and the time this has been on central [Risks and why]: this particular patch follows up the other two on this bug for another edge case. Elsewhere, Ehsan said: "The risk on this patch set is moderate. We have tried to not change the existing behavior except for only the well known cases, but this code is really old and not very well understood. I (ehsan) would like these fixes to be backported on Aurora as soon as we can to get broader testing, and will be on the hook for fixing any regressions found." [String/UUID change made/needed]: none

Comment on attachment 8553251 [details] [diff] [review] Don't depend on the editor in nsDocumentEncoder Prereq required for bug 1119503. Aurora+

https://hg.mozilla.org/releases/mozilla-aurora/rev/f76c1b5753c6 https://hg.mozilla.org/releases/mozilla-aurora/rev/05d65b2c6a3d https://hg.mozilla.org/releases/mozilla-aurora/rev/e0de9ceed8b3