832025 - Major regression in HTML editing rules in CSS mode related to TypeInState

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Steps to reproduce:

1. open the Midas demo http://www-archive.mozilla.org/editor/midasdemo/

2. click *twice* on the already checked "Use CSS" checkbox otherwise CSS
   mode is not correctly selected

3. type some text in editable area, do NOT type a carriage return

4. use dropdown menu to turn text from "normal" into "Header 1"

5. press the Enter key to create a paragraph after h1

6. type some text

Expected result: text is not bold
Actual result:   text is bold !!!

This is a _major_ regression severely impacting *all* embedders of the editor.
In particular, this is a show-stopper for BlueGriffon v1.6 and should
probably be a blocker for richtext emails in next Thunderbird....
Sorry guys:-(

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

So it's most probably related to cached styles. A regression
has probably stopped triggering a call to ClearCachedStyles()
in the case of ReturnInHeader(). I'll investigate a bit more
tomorrow morning, it's already late in the night here.

Comment 2

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

This bug will also severely impact all inline wysiwyg editors, for instance
Wikipedia's inline wysiwyg editor, CKEditor and others. In other
words, hundreds of major web sites.

Comment 3

[(no longer active)]

Alice, any chance you could please help us find a regression range here?  Thanks a lot!

Comment 4

[(no longer active)]

(In reply to comment #1)
> So it's most probably related to cached styles. A regression
> has probably stopped triggering a call to ClearCachedStyles()
> in the case of ReturnInHeader(). I'll investigate a bit more
> tomorrow morning, it's already late in the night here.

I would appreciate that!

Comment 5

[Matthias Versen [:Matti]]

Last good nightly: 2012-05-18
First bad nightly: 2012-05-19

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e794cef56df6&tochange=642d1a36702f

bug 590640 ?

Comment 6

[Alice0775 White]

Umm. I cannot reproduce the problem in Firefox15,16,  esr17, 18, 19beta, Aurora20.0a2 and Nightly21.0a1.
 AFAICT, This was already fixed by Bug 780035.

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/11780e80c8c3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517220915
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e8ebc8f1825e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517232316
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=11780e80c8c3&tochange=e8ebc8f1825e
Regressed by: 590640


Fixed  window(m-c)
Bad:
http://hg.mozilla.org/mozilla-central/rev/16932b475002
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120814150602
Fixed:
http://hg.mozilla.org/mozilla-central/rev/86ee4deea55b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120814175101
Fixed Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=16932b475002&tochange=86ee4deea55b


Fixed  window(m-i)
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/4110062a8d78
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120814022043
Fixed:
http://hg.mozilla.org/integration/mozilla-inbound/rev/59707ed19e48
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120814030522
Fixed Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4110062a8d78&tochange=59707ed19e48
Fixed by: Bug 780035

Comment 7

[Alice0775 White]

Oops, sorry. My bad:

Bug 780035 was fixed font size only.
And yes, The bold stye is still broken.

Comment 8

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Fix in hand! Now writing a test for it, stay tuned.

Comment 9

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: trivial patch + test

Trivial patch with corresponding test. Feel free to test/check-in for
me, I'm working fulltime on BlueGriffon 1.6 RC2 and invested time here
only because it was a show-stopper for BlueGriffon.

Comment 10

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

So the guilty code was Aryeh's in bug 590640 when he introduced
IsStyleCachePreservingAction(). This returns true for EditAction::insertBreak
but he forgot insertBreak can happen at the end of a block or inside it, the
behaviour at the end of a block being a bit different for headers, lists, pre
in some cases...
I just realize we have a similar bug at the end of lists when a double-CRs
breaks the list and creates a new paragraph. Let me update my patch and add
a new test.

Comment 11

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: patch take #2 + tests

The patch fixes both return-in-header and return-in-listitem. Two tests
for that are included. Again, feel free to try and check-in for me. Thanks.

Comment 12

[(no longer active)]

Comment on attachment 703889 [details] [diff] [review]
patch take #2 + tests

Thanks for your patch!

https://tbpl.mozilla.org/?tree=Try&rev=3e57edaa00c5

Comment 13

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a294f877514

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8a294f877514