Bug 30148 – Applying h1 format to text node after HR tag fails to include first letter of text node.

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Michael Thomas]

STEPS TO REPRODUCE:
1. Go to midas demo: http://www.mozilla.org/editor/midasdemo/
2. Enter the following HTML: '<hr>test'
3. Place the cursor after the word 'test'.
4. Select 'Heading 1' from the menu.

RESULT:
The heading style is applied to the entire line except the first character:

<hr><h1><span class="Apple-style-span" style="font-weight: normal; font-size: medium; ">t</span>est</h1>

EXPECTED:
The heading style is applied to the entire line:
<hr><h1>test</h1>

Automated test case:

http://www.plexode.com/cgi-bin/eval3.py#ht=%3Cdiv%20id%3D%22editor%22%20contenteditable%3Dtrue%3E%3Chr%3Etest%3C%2Fdiv%3E%0A&ohh=1&ohj=1&jt=var%20div%20%3D%20document.getElementById('editor')%3B%0A%0A%2F%2F%20Change%20this%20to%20select%20a%20range%0AselectRange(div.childNodes%5B1%5D%2C%204%2C%20div.childNodes%5B1%5D%2C%204)%3B%0A%0A%2F%2F%20Change%20this%20to%20run%20execCommand%0Adocument.execCommand('FormatBlock'%2C%20false%2C%20'%3Ch1%3E')%3B%0A%0Afunction%20selectRange(startNode%2C%20startOffset%2C%20endNode%2C%20endOffset)%20%7B%0A%20%20var%20sel%20%3D%20window.getSelection()%3B%0A%20%20var%20range%20%3D%20null%3B%0A%20%20try%20%7B%0A%20%20%20%20range%20%3D%20sel.getRangeAt(0)%3B%0A%20%20%7D%20catch(ex)%20%7B%0A%20%20%20%20range%20%3D%20document.createRange()%3B%0A%20%20%7D%0A%20%20range.setStart(startNode%2C%20startOffset)%3B%0A%20%20range.setEnd(endNode%2C%20endOffset)%3B%0A%20%20sel.removeAllRanges()%3B%0A%20%20sel.addRange(range)%3B%0A%7D&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1

[Tony Chang]

The patch on bug 32131  fixes this test as well.  It's another case where we call applyStyle then restore the selection incorrectly.  In this case, we're off by a character so the selection no longer contains the first letter after the HR tag.

[Tony Chang]

I think this bug is now fixed, but we should add a layout test so it doesn't regress.

[Ryosuke Niwa]

Created attachment 50354 [details]
adds a test case

We could make this a js test and add more test cases.

[Darin Adler]

Comment on attachment 50354 [details]
adds a test case

> +This tests removing underline which is applied outside of unsplittable element. We shouldn't be removing underline.

Huh?

> +<p>This tests removing underline which is applied outside of unsplittable element.  We shouldn't be removing underline.</p>

Wrong description here.

[Ryosuke Niwa]

Created attachment 50364 [details]
fixed the test description

(In reply to comment #4)
> (From update of attachment 50354 [details])
> > +<p>This tests removing underline which is applied outside of unsplittable element.  We shouldn't be removing underline.</p>
> 
> Wrong description here.

Sorry, I was too careless.  Fixed.

[Ryosuke Niwa]

Comment on attachment 50364 [details]
fixed the test description

Added to the commit queue.

[WebKit Commit Bot]

Comment on attachment 50364 [details]
fixed the test description

Clearing flags on attachment: 50364

Committed r55876: <http://trac.webkit.org/changeset/55876>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.