Bug 23366

Summary: Need to test toggle behavior of editing style commands
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: deepak.jindal, jparent, justin.garcia, ojan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 17733, 20215, 22810    
Attachments:
Description Flags
the js portion of the test (goes in LayoutTests/editing/execCommand/resources
none
Add toggle-styles test for editing. ap: review+

Description Eric Seidel (no email) 2009-01-15 16:43:37 PST
Need to test toggle behavior of editing style commands

I've wrote a test to cover bug 17733, bug 20215, and bug 22810, which I'll attach here.
Comment 1 Eric Seidel (no email) 2009-01-15 16:45:21 PST
Created attachment 26774 [details]
the js portion of the test (goes in LayoutTests/editing/execCommand/resources

Current Safari results:
Test to make sure styles toggle as expected and tag-based styles can be removed by editing commands.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS document.compatMode is "BackCompat"
PASS bold toggle
PASS bold removing b
FAIL bold removing font-weight: bold -- <span style="">test</span>
PASS italic toggle
PASS italic removing i
FAIL italic removing font-style: italic -- <span style="">test</span>
FAIL subscript toggle: [object HTMLElement]
FAIL subscript removing sub -- <sub>test</sub>
FAIL subscript removing vertical-align: subscript -- <span><span class="Apple-style-span" style="vertical-align: sub;">test</span></span>
FAIL superscript toggle: [object HTMLElement]
FAIL superscript removing sup -- <sup>test</sup>
FAIL superscript removing vertical-align: superscript -- <span><span class="Apple-style-span" style="vertical-align: super;">test</span></span>
PASS strikethrough toggle
FAIL strikethrough removing s -- <s style=""><span class="Apple-style-span" style="text-decoration: none;">test</span></s>
FAIL strikethrough removing text-decoration: line-through -- <span style="">test</span>
FAIL strikethrough removing strike -- <strike style=""><span class="Apple-style-span" style="text-decoration: none;">test</span></strike>
PASS underline toggle
FAIL underline removing u -- <u style=""><span class="Apple-style-span" style="text-decoration: none;">test</span></u>
FAIL underline removing text-decoration: underline -- <span style="">test</span>
PASS successfullyParsed is true

TEST COMPLETE

Current FF3 results:

Test to make sure styles toggle as expected and tag-based styles can be removed by editing commands.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS document.compatMode is "BackCompat"
PASS bold toggle
PASS bold removing b
PASS bold removing font-weight: bold
PASS italic toggle
PASS italic removing i
PASS italic removing font-style: italic
PASS subscript toggle
PASS subscript removing sub
FAIL subscript removing vertical-align: subscript -- <sub><span style="">test</span></sub>
PASS superscript toggle
PASS superscript removing sup
FAIL superscript removing vertical-align: superscript -- <sup><span style="">test</span></sup>
PASS strikethrough toggle
FAIL strikethrough removing s -- <s>test</s>
PASS strikethrough removing text-decoration: line-through
PASS strikethrough removing strike
PASS underline toggle
PASS underline removing u
PASS underline removing text-decoration: underline
PASS successfullyParsed is true

TEST COMPLETE
Comment 2 Eric Seidel (no email) 2009-01-15 16:46:27 PST
I'm impressed that FF only fails subscript, superscript and <s> for strikethrough.  I've not tried Opera or IE yet.
Comment 3 Eric Seidel (no email) 2009-01-15 16:50:46 PST
I would like to know from you editing gurus if I need to be testing for removal of any additional tags.

These come to mind:
<big>
<center>
<em>
<small>
<strong>
<tt>

I'd like to make a complete test case before I go trying to make sure we support these equivalencies.  After we support the equivalencies and toggling correctly we can look at implementing firefox's "css mode" for execCommand
Comment 4 Ojan Vafai 2009-01-16 10:03:37 PST
em and strong for sure should be tested as well. I don't know if you can actually get any of the other tags from existing execCommands in any browser. Of the top of my head, it seems like you have the complete list here.

There are other execCommands that aren't quite toggles that need testing, but maybe it should be a different bug (e.g. JustifyRight followed by JustifyNone, or Indent followed by Outdent).
Comment 5 Eric Seidel (no email) 2009-01-16 11:57:33 PST
Created attachment 26803 [details]
Add toggle-styles test for editing.

 LayoutTests/ChangeLog                              |   24 +++++
 .../editing/execCommand/resources/toggle-styles.js |   97 ++++++++++++++++++++
 .../editing/execCommand/toggle-styles-expected.txt |   29 ++++++
 LayoutTests/editing/execCommand/toggle-styles.html |   13 +++
 4 files changed, 163 insertions(+), 0 deletions(-)
Comment 6 Alexey Proskuryakov 2009-01-16 12:08:13 PST
Comment on attachment 26803 [details]
Add toggle-styles test for editing.

> +        ():

Please help prepare-ChangeLog by correcting its mistake.

We usually prefer not to land FAIL results, but Eric says he's going to fix these soon anyway. r=me
Comment 7 Justin Garcia 2009-01-16 12:18:13 PST
 i can't find the output for testTagRemovalOnToggle("strong", "bold");, there should be a "PASS bold removing strong" in the results, right?
Comment 8 Eric Seidel (no email) 2009-01-16 13:52:44 PST
(In reply to comment #7)
>  i can't find the output for testTagRemovalOnToggle("strong", "bold");, there
> should be a "PASS bold removing strong" in the results, right?

The results were wrong in that patch, but were landed correctly.  We FAIL to remove strong (then again, so does FF).  Thanks!  (And sorry I missed you on IRC, I was @ lunch).
Comment 9 Eric Seidel (no email) 2009-01-16 13:53:53 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/editing/execCommand/resources/toggle-styles.js
	A	LayoutTests/editing/execCommand/toggle-styles-expected.txt
	A	LayoutTests/editing/execCommand/toggle-styles.html
Committed r39986