Bug 27858

Summary: selectionHasStyle doesn't handle text-specific properties properly
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, justin.garcia, tonikitoo
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 27865, 28055    
Bug Blocks: 23892    
Attachments:
Description Flags
fixes the bug, large because converts a pixel test to a dumpastext.
none
fixed per comment on IRC
none
fixes the bug
none
resubmission after split
eric: review+
reverts the behavioral change in Mac
none
fixes the bug and changes the WebKit behavior on all platforms except mac justin.garcia: review+

Ryosuke Niwa
Reported 2009-07-30 15:20:42 PDT
selectionHasStyle does not return TrueTriState when asked if <b><span style="text-decoration: underline;">hello</span><span style="text-decoration: underline;">world</span></b> is has underline. Because b does not have underline style, it returns MixedTriState. It also does not take care of -webkit-text-decorations; i.e. text decorations added by u, s, and strike.
Attachments
fixes the bug, large because converts a pixel test to a dumpastext. (64.12 KB, patch)
2009-07-30 17:41 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
fixed per comment on IRC (63.18 KB, patch)
2009-07-30 18:38 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
fixes the bug (61.22 KB, patch)
2009-08-04 18:17 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
resubmission after split (11.48 KB, patch)
2009-08-06 17:38 PDT, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
reverts the behavioral change in Mac (14.16 KB, patch)
2009-08-07 13:49 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
fixes the bug and changes the WebKit behavior on all platforms except mac (18.41 KB, patch)
2009-08-07 14:21 PDT, Ryosuke Niwa 		justin.garcia: review+
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2009-07-30 17:41:59 PDT
Created attachment 33849 [details] fixes the bug, large because converts a pixel test to a dumpastext.
Ryosuke Niwa
Comment 2 2009-07-30 18:38:21 PDT
Created attachment 33852 [details] fixed per comment on IRC
Ryosuke Niwa
Comment 3 2009-07-30 18:39:44 PDT
Will fix // CSS properties that only has a visual difference when applied to text. to // CSS properties that have a visual difference only if applied to text.
Ryosuke Niwa
Comment 4 2009-07-30 18:42:11 PDT
Comment on attachment 33852 [details] fixed per comment on IRC will split into two patches.
Ryosuke Niwa
Comment 5 2009-08-04 18:17:49 PDT
Created attachment 34107 [details] fixes the bug
Eric Seidel (no email)
Comment 6 2009-08-06 11:53:57 PDT
Comment on attachment 34107 [details] fixes the bug Converting the tests in one patch and then changing them in a second would be clearer.
Ryosuke Niwa
Comment 7 2009-08-06 14:15:06 PDT
The test conversion bug is filed as https://bugs.webkit.org/show_bug.cgi?id=28055 and the corresponding patch has been submitted.
Ryosuke Niwa
Comment 8 2009-08-06 17:38:36 PDT
Created attachment 34236 [details] resubmission after split
Eric Seidel (no email)
Comment 9 2009-08-06 19:15:07 PDT
Comment on attachment 34236 [details] resubmission after split Looks OK.
Justin Garcia
Comment 10 2009-08-06 22:44:48 PDT
Comment on attachment 34236 [details] resubmission after split + WebKit now applies the text styles (bold, italic, etc) if the specified style was not present on + at least one text node, rather than at the beginning of selection. This change is tested in toggle-compound-styles. Sorry I didn't notice this until now but we behaved this way to match Mac OS X editing behavior (TextEdit). Perhaps we should preserve the old behavior for Mac-only.
Ryosuke Niwa
Comment 11 2009-08-07 13:49:33 PDT
Created attachment 34325 [details] reverts the behavioral change in Mac
Ryosuke Niwa
Comment 12 2009-08-07 14:21:31 PDT
Created attachment 34329 [details] fixes the bug and changes the WebKit behavior on all platforms except mac
Ryosuke Niwa
Comment 13 2009-08-07 14:25:17 PDT
mn... I don't know why it has svn property change. that doesn't seem right. should i delete that property change before I commit?
Ryosuke Niwa
Comment 14 2009-08-07 16:06:43 PDT
Landed in http://trac.webkit.org/changeset/46920.
Note You need to log in before you can comment on or make changes to this bug.