Summary: | queryCommandState('underline') returns false if the selected text is also bold and italic | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Petersen <c.petersen87> | ||||||||
Component: | HTML Editing | Assignee: | 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
Chris Petersen
2006-09-25 08:59:35 PDT
This issue is covered in <rdar://problem/4748375> *** Bug 11023 has been marked as a duplicate of this bug. *** Looks like it's fixed for bold and italic. Still broken for underline, though. > 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.
*** Bug 20089 has been marked as a duplicate of this bug. *** 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. Created attachment 35089 [details]
demo
Created attachment 35091 [details]
fixes the bug, two lines of change in EditorsCommand.cpp and adds a test
Comment on attachment 35091 [details]
fixes the bug, two lines of change in EditorsCommand.cpp and adds a test
r=me
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! 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
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 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. 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. (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. |