Bug 7360 – Typing style isn't bold in a bold context

• Component: HTML Editing
• Status: NEW

[Graham Dennis]

Let me prefix this by saying that I'm not sure that this actually is a bug.
If one adds styling to the text at the start of a contentEditable div, and then deletes all of the content in the div, any new text that is added has the styling that was added to the text at the start of the div. This is the same behaviour as TextEdit, however, for TextEdit, there is no well-defined default style. For a contentEditable div, it is possible that a style has been applied to the div, which has since been overridden by the span. It makes sense that there should be a way to remove the style applied by the span. One way to do this would be to remove a span once it contains no text.

I'll attach a testcase.

[Graham Dennis]

Created attachment 6607 [details]
testcase

The red colour is applied by a <span> element inside the contentEditable div. If all the text in the div is deleted, this style is still applied to any new text typed in the div.

[Joost de Valk (AlthA)]

The behavior i can confirm... Don't know if this is wrong thoug... Justin, what do you think?

[Justin Garcia]

(In reply to comment #0)
> This is the same behaviour as TextEdit, however, for TextEdit, there is no well-defined default
> style. 

In TextEdit, create a few lines of bold text, with an empty line somewhere in the middle.  Click in that empty line, unbold with command-b, and start typing.  You'll get unbolded text.  Now, delete all the text on that line, and begin typing again.  You'll still have unbolded text.  So, even though you're inside a bold context, the unbold style hangs onto the caret even though you've deleted all unbold text.

Also notice that if you click somewhere else and then click back on the empty line and start typing, you'll get bold text.  So clicking away and clicking back blows away the unbold style that was hanging on the caret.  We mimic this behavior as well.

We consider this behavior correct, since our goal in general has been to mimic the behavior of TextEdit.

[Graham Dennis]

(In reply to comment #3)
> Also notice that if you click somewhere else and then click back on the empty
> line and start typing, you'll get bold text.  So clicking away and clicking
> back blows away the unbold style that was hanging on the caret.  We mimic this
> behavior as well.

I just tried to do this, but the behaviour is not what you said. If in the testcase, I create three lines of text, with a blank line in the middle, then bold the lot, text on the blank line is typed as bold. So far, so good. I unbold and type text which is unbolded. Then I delete the the text on this line (but not the line itself), then click on the URL entry field, then back on the empty line. New text I type is not bold, but we would expect bold text (and TextEdit does show bold text).

I haven't reopened this bug in case I'm not understanding this properly.

[Justin Garcia]

(In reply to comment #4)
> (In reply to comment #3)
> > Also notice that if you click somewhere else and then click back on the empty
> > line and start typing, you'll get bold text.  So clicking away and clicking
> > back blows away the unbold style that was hanging on the caret.  We mimic this
> > behavior as well.
> 
> I just tried to do this, but the behaviour is not what you said. If in the
> testcase, I create three lines of text, with a blank line in the middle, then
> bold the lot, text on the blank line is typed as bold. So far, so good. I
> unbold and type text which is unbolded. Then I delete the the text on this line
> (but not the line itself), then click on the URL entry field, then back on the
> empty line. New text I type is not bold, but we would expect bold text (and
> TextEdit does show bold text).

You're right, this is a bug.  Reopening.

[Graham Dennis]

I've had a look at this, and the problem seems to be caused when DeleteSelectionCommand::calculateTypingStyleAfterDelete applies the typing style to the placeholder element. However, I can't figure out how to calculate the correct style. Consider the following two cases:
1: Type three lines, bold all three, and delete the text in the middle line.
2: Type three lines, bold just the first and third lines, and delete the text in the middle line.
In case 1, after moving the caret away and back again, new text on the second line should be bold. In case 2, after moving the caret away and back again, new text on the second line should not be bold. In HTML, it will be difficult to tell the difference between case 1 and case 2 because all tags must be nested, and because the newlines aren't <br/>'s, but each paragraph is a block element, there must be a boldface tag for each line separately. So how can we tell the difference between these cases in HTML? For the given two cases, it looks like one solution might be to use the current typing style (this is what is done in the code at present), but consider a third case:
3: Type three lines, bold all three, unbold the middle line, now delete the text in the middle line.
In this case, after clicking away and back again, the text should be bold, which is not the previous typing style.
Ideas anyone?

[Graham Dennis]

Created attachment 7526 [details]
work in progress patch

Justin,
I've been doing a bit of work on this bug, and what I have been trying to do is to apply styles to the block element when a style should be applied to the entire paragraph (i.e. the style should be persistent when moving the caret away and back to the paragraph). This means that some styles aren't using the apple-style-span span's... Is this reasonable behaviour?
The patch I have attached is a work in progress that I'd like your feedback on. I haven't marked it r? as it is missing a testcase, changelog entry, etc. I just want to know if you think I'm on the right track. Other than that, this patch does fix this bug.

[Justin Garcia]

I'm not sure that this bug is titled appropriately anymore, and the testcase that's attached doesn't describe what it's testing.  Let's make sure we're on the same page.  There is only one bug discussed here that I am familiar with.  If there are others, let's file those in separate reports.

Here's the tree we produce, and the typing style resulting from the three examples you gave:

> 1: Type three lines, bold all three, and delete the text in the middle line.

<div><b>line 1</b></div>
<div><b><br></b></div>
<div><b>line 3</b></div>

Current typing style: bold

Typing on the second line produces characters with the right style, and clicking away/clicking back and typing produces characters with the right style.


> 2: Type three lines, bold just the first and third lines, and delete the text in the middle line.

<div><b>line 1</b></div>
<div><br></div>
<div><b>line 3</b></div>

Current typing style: unbold

Typing on the second line produces characters with the right style, and clicking away/clicking back and typing produces characters with the right style.


> 3: Type three lines, bold all three, unbold the middle line, now delete the text in the middle line.

<div><b>line 1</b></div>
<div><br></div>
<div><b>line 3</b></div>

Current typing style: unbold

Typing on the second line produces characters with the right style, but clicking away/clicking back and typing produces unbold text.  This is the only bug as far as I can tell.  I'm going to retitle this bug and add a testcase that demonstrates this bug.


> In HTML, it will be difficult to tell the difference between case 1 and case 2 because all tags must be nested

What do you mean by "all tags must be nested"?

[Justin Garcia]

Created attachment 7528 [details]
testcase

This is an automated testcase that demonstrates this bug:
Type three lines of text
Bold them all
Unbold the middle line
Delete the text on the middle line
Click away and click back to the middle line
Start typing

You should see bold text.  You instead see unbold text.

[Graham Dennis]

Sorry for the delay in replying...

(In reply to comment #8)
> I'm not sure that this bug is titled appropriately anymore, and the testcase
> that's attached doesn't describe what it's testing.  Let's make sure we're on
> the same page.  There is only one bug discussed here that I am familiar with. 
> If there are others, let's file those in separate reports.

That's the bug I'm talking about.

Of the three cases I was talking about, only the third worked incorrectly. The reason I mentioned the other two is because they are similar, and at present the HTML code produced for 2 and 3 is identical, however I wasn't sure how they should differ when the bug is fixed.

> > In HTML, it will be difficult to tell the difference between case 1 and case 2 because all tags must be nested
> 
> What do you mean by "all tags must be nested"?
What I mean to say is that in Cocoa, this problem is avoided because we can say that the bolding started in one paragraph, continued through one paragraph and into the next. However this is impossible to do with one tag in HTML as all tags must be nested. The bolding region that starts in the first paragraph must also finish in the first paragraph. So a new bolding region must be used for the second and third paragraphs.

My solution to this problem is where a bolding region (or styling region in general) would cover an entire paragraph (actually any Block element), the style is applied to the block element itself, instead of creating a styling span within it. This way, when all text in a block is deleted (and any empty styling spans removed), when the cursor moves away and back again, the style that was applied to the block will still apply to the text.

Let me know if I haven't explained myself properly...

[Justin Garcia]

> This way, when all text in a block is deleted (and any empty
> styling spans removed), when the cursor moves away and back again, the style
> that was applied to the block will still apply to the text.

So, would it be better to fix this bug by not removing a span when its contents are removed?

Please add some testcases. 

     // Do not check for legacy styles here. Those styles, like <B> and <I>, only apply for
     // inline content.
+    // No longer true.

Just remove the comment.

r- for now until you add some testcases.

[Justin Garcia]

+        // If the node we are in is an empty paragraph, then we have selected the entire paragraph
+        // so we want to apply the style to the whole paragraph
+        VisiblePosition startPos(start, VP_UPSTREAM_IF_POSSIBLE);
+        if (start.offset() == end.offset() && isStartOfParagraph(startPos) && isEndOfParagraph(startPos)) {
+            HTMLElement *block = static_cast<HTMLElement *>(start.node()->enclosingBlockFlowElement());
+            removeCSSStyle(style, block);
+            addBlockStyleIfNeeded(style, block);

Just because startPos is in an empty paragraph, doesn't mean that it's in a block by itself.  For example [br2, 0] example in the tree below:
<div><br1><br2></div>

[Mark Rowe (bdash)]

Comments indicates this has been confirmed as a bug.

[Aryeh Gregor]

(In reply to comment #10)
> My solution to this problem is where a bolding region (or styling region in general) would cover an entire paragraph (actually any Block element), the style is applied to the block element itself, instead of creating a styling span within it. This way, when all text in a block is deleted (and any empty styling spans removed), when the cursor moves away and back again, the style that was applied to the block will still apply to the text.

This is not a good idea.  Gecko does something like this, and it causes problems:

1) It doesn't work at all if styleWithCSS is false.  You can't use CSS for styling in that case: it would break non-CSS UAs, like some mail clients.  (Or so I've been told.)

2) If you do it only when styleWithCSS is true, it will produce markup that has different effects in some cases.  For instance, <p style="text-decoration:underline"><font color=red>foo</font></p> has a different underline color than <p style="color:red; text-decoration:underline">foo</p>.  This is bad, because styleWithCSS vs. not should only affect the markup produced, not the visual effect.

3) It breaks hiliteColor, because background-color does something different on block vs. inline elements.  <p style="background-color:teal">foo</p> is different from <p><span style="background-color:teal">foo</span></p>.

4) It makes everything more complicated.  You'll have to add lots of different code paths everywhere to properly handle the two different types of style.

Instead, I would suggest that if the entire contents of the block are deleted, the styling tag should just be left.  So instead of <p><b>foo</b></p> -> <p><br></p> with a typing style set, it should become <p><b><br></b></p> with no typing style.

This bug also affects the editing spec.  I filed a bug against that:

http://www.w3.org/Bugs/Public/show_bug.cgi?id=13831

I plan to specify the solution that I just outlined, if no one has any objections to it.

[Ryosuke Niwa]

Also see the bug 34608.

[Brent Fulgham]

Chrome and Safari agree on behavior (the middle line is not bolded), while Firefox shows all three lines in bold.

[Radar WebKit Bug Importer]

<rdar://problem/96544639>