Bug 11022

Summary: queryCommandState('underline') returns false if the selected text is also bold and italic
Product: WebKit Reporter: Chris Petersen <c.petersen87>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, futurama, josh, justin.garcia, rniwa
Priority: P2 Keywords: GoogleBug, InRadar
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 9638    
Attachments:
Description Flags
demo
none
fixes the bug, two lines of change in EditorsCommand.cpp and adds a test
darin: review+, eric: commit-queue-
test case is fixed (one line change: 19@text-decoration-state.js) darin: review+

Chris Petersen
Reported 2006-09-25 08:59:35 PDT
* SUMMARY Inserting a caret into styled text (bold, italic, underline) doesn't highlight the associated toolbar buttons (bold, italic, underline) * STEPS TO REPRODUCE 1. With TOT Webkit r-16541 , login to your gmail account 2. Create a new message (Rich Formatting) 3. Place caret in message body and and click "B", "I" and "U" . Notice the button are highlighted when pressed. 4. Type out "This is some text". Verify this text has all three styles applied. 5. With this caret at the end of this text, press the return to create a new line 6. Place the caret at the end of "This is some text" and press the return key. This will place the caret on a new line. 7. Click "B", "I", and "U" to remove this style then start to type some text 8. Now, click in the line above which contains styling on the text. Notice , the B, U, I buttons don't become highlighted. * RESULTS Style buttons on the toolbar should become highlighted if caret is inserted into style text but isn't.
Attachments
demo (574 bytes, text/html)
2009-08-18 17:31 PDT, Ryosuke Niwa 		no flags
Details
fixes the bug, two lines of change in EditorsCommand.cpp and adds a test (8.45 KB, patch)
2009-08-18 17:42 PDT, Ryosuke Niwa 		darin: review+
eric: commit-queue-
DetailsFormatted DiffDiff
test case is fixed (one line change: 19@text-decoration-state.js) (8.48 KB, patch)
2009-08-19 17:31 PDT, Ryosuke Niwa 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Petersen
Comment 1 2006-09-25 09:01:25 PDT
This issue is covered in <rdar://problem/4748375>
Justin Garcia
Comment 2 2006-10-31 15:54:52 PST
*** Bug 11023 has been marked as a duplicate of this bug. ***
Justin Garcia
Comment 3 2008-06-19 18:41:24 PDT
Looks like it's fixed for bold and italic. Still broken for underline, though.
Justin Garcia
Comment 4 2008-06-19 18:44:34 PDT
> Looks like it's fixed for bold and italic. Still broken for underline, though. Underline seems to only have problems when the text has is also Bold and Italic.
Justin Garcia
Comment 5 2009-02-17 16:16:52 PST
*** Bug 20089 has been marked as a duplicate of this bug. ***
Ryosuke Niwa
Comment 6 2009-08-18 15:25:42 PDT
This bug is caused by us checking text-decoration property. We should be checking -webkit-text-decorations-in-effect. It's literally two-line change for underline and line-through. I'll submit a patch after writing some tests.
Ryosuke Niwa
Comment 7 2009-08-18 17:31:17 PDT
Created attachment 35089 [details] demo
Ryosuke Niwa
Comment 8 2009-08-18 17:42:49 PDT
Created attachment 35091 [details] fixes the bug, two lines of change in EditorsCommand.cpp and adds a test
Darin Adler
Comment 9 2009-08-18 17:52:43 PDT
Comment on attachment 35091 [details] fixes the bug, two lines of change in EditorsCommand.cpp and adds a test r=me
Ryosuke Niwa
Comment 10 2009-08-18 17:55:00 PDT
Comment on attachment 35091 [details] fixes the bug, two lines of change in EditorsCommand.cpp and adds a test (In reply to comment #9) > (From update of attachment 35091 [details]) > r=me Thanks, Darin!
Eric Seidel (no email)
Comment 11 2009-08-18 18:08:12 PDT
Comment on attachment 35091 [details] fixes the bug, two lines of change in EditorsCommand.cpp and adds a test Rejecting patch 35091 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Eric Seidel (no email)
Comment 12 2009-08-18 19:00:38 PDT
Testing 11102 test cases. editing/style/text-decoration-state.html -> failed Exiting early after 1 failures. 4342 tests run. 73.44s total testing time 4341 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 1 test case (<1%) had stderr output
Ryosuke Niwa
Comment 13 2009-08-19 17:31:59 PDT
Created attachment 35165 [details] test case is fixed (one line change: 19@text-decoration-state.js) The original patch had a bug in the test case.
Darin Adler
Comment 14 2009-08-19 17:40:46 PDT
Comment on attachment 35165 [details] test case is fixed (one line change: 19@text-decoration-state.js) > + * editing/EditorCommand.cpp: > + (WebCore::stateStrikethrough): ditto > + (WebCore::stateUnderline): ditto This is a little mysterious. Usually "ditto" means "repeat the comment above". In this case the only comment above is the summary one describing the entire change. I'd suggest writing something other than "ditto" here in patches in the future. I would have written: (WebCore::stateStrikethrough): Use -webkit-test-decorations-in-effect instead of text-decoration. (WebCore::stateUnderline): Ditto.
Ryosuke Niwa
Comment 15 2009-08-19 19:17:01 PDT
(In reply to comment #14) > (From update of attachment 35165 [details]) > > + * editing/EditorCommand.cpp: > > + (WebCore::stateStrikethrough): ditto > > + (WebCore::stateUnderline): ditto > > This is a little mysterious. Usually "ditto" means "repeat the comment above". > In this case the only comment above is the summary one describing the entire > change. I'd suggest writing something other than "ditto" here in patches in the > future. > > I would have written: > > (WebCore::stateStrikethrough): Use -webkit-test-decorations-in-effect > instead of text-decoration. > (WebCore::stateUnderline): Ditto. Fixed and committed as http://trac.webkit.org/changeset/47541.
Note You need to log in before you can comment on or make changes to this bug.