Bug 27858 - selectionHasStyle doesn't handle text-specific properties properly
Summary: selectionHasStyle doesn't handle text-specific properties properly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P3 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on: 27865 28055
Blocks: 23892
  Show dependency treegraph
 
Reported: 2009-07-30 15:20 PDT by Ryosuke Niwa
Modified: 2010-06-28 10:46 PDT (History)
3 users (show)

See Also:


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 Details | Formatted Diff | Diff
fixed per comment on IRC (63.18 KB, patch)
2009-07-30 18:38 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
fixes the bug (61.22 KB, patch)
2009-08-04 18:17 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
resubmission after split (11.48 KB, patch)
2009-08-06 17:38 PDT, Ryosuke Niwa
eric: review+
Details | Formatted Diff | Diff
reverts the behavioral change in Mac (14.16 KB, patch)
2009-08-07 13:49 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
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+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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.
Comment 1 Ryosuke Niwa 2009-07-30 17:41:59 PDT
Created attachment 33849 [details]
fixes the bug, large because converts a pixel test to a dumpastext.
Comment 2 Ryosuke Niwa 2009-07-30 18:38:21 PDT
Created attachment 33852 [details]
fixed per comment on IRC
Comment 3 Ryosuke Niwa 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.
Comment 4 Ryosuke Niwa 2009-07-30 18:42:11 PDT
Comment on attachment 33852 [details]
fixed per comment on IRC

will split into two patches.
Comment 5 Ryosuke Niwa 2009-08-04 18:17:49 PDT
Created attachment 34107 [details]
fixes the bug
Comment 6 Eric Seidel (no email) 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.
Comment 7 Ryosuke Niwa 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.
Comment 8 Ryosuke Niwa 2009-08-06 17:38:36 PDT
Created attachment 34236 [details]
resubmission after split
Comment 9 Eric Seidel (no email) 2009-08-06 19:15:07 PDT
Comment on attachment 34236 [details]
resubmission after split

Looks OK.
Comment 10 Justin Garcia 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.
Comment 11 Ryosuke Niwa 2009-08-07 13:49:33 PDT
Created attachment 34325 [details]
reverts the behavioral change in Mac
Comment 12 Ryosuke Niwa 2009-08-07 14:21:31 PDT
Created attachment 34329 [details]
fixes the bug and changes the WebKit behavior on all platforms except mac
Comment 13 Ryosuke Niwa 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?
Comment 14 Ryosuke Niwa 2009-08-07 16:06:43 PDT
Landed in http://trac.webkit.org/changeset/46920.