Bug 43465 - fontColorChangesComputedStyle, fontSizeChangesComputedStyle, and fontFaceChangesComputedStyle should be removed
Summary: fontColorChangesComputedStyle, fontSizeChangesComputedStyle, and fontFaceChan...
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Ryosuke Niwa
Depends on:
Reported: 2010-08-03 21:54 PDT by Ryosuke Niwa
Modified: 2010-08-09 17:24 PDT (History)
3 users (show)

See Also:

Patch (4.20 KB, patch)
2010-08-03 22:10 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch (9.22 KB, patch)
2010-08-03 23:52 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 2010-08-03 21:54:42 PDT
Since StyleChange class removes the styles that are already present at the position, fontColorChangesComputedStyle, fontSizeChangesComputedStyle, and fontFaceChangesComputedStyle are redundant and should be removed.

Note that fontColorChangesComputedStyle uses visited dependent information, and it's a potential security hole.
Comment 1 Ryosuke Niwa 2010-08-03 22:10:22 PDT
Created attachment 63415 [details]
Comment 2 Justin Garcia 2010-08-03 22:22:07 PDT
Comment on attachment 63415 [details]

r=me assuming you verified that that bug that Beth mentioned didn't regress.
Comment 3 Justin Garcia 2010-08-03 22:23:15 PDT
Ah, I just read your email, thanks Ryosuke!
Comment 4 Ryosuke Niwa 2010-08-03 23:52:20 PDT
Created attachment 63421 [details]
Comment 5 Ryosuke Niwa 2010-08-03 23:56:28 PDT
(In reply to comment #3)
> Ah, I just read your email, thanks Ryosuke!

Justin, I just figured out that my change will cause redundant font tag to be added in the reproduction steps Beth sent me.  Although this won't the bug Beth fixed to reappear, I added a fix for this problem which is basically to add a work-around for the 28282 just as we did for StyleChange. I also added a test case.  Sorry for the trouble.  I always realize problems with my patch after I posted it on bugzilla :(
Comment 6 Justin Garcia 2010-08-09 16:35:58 PDT
Comment on attachment 63421 [details]

Looks good, r=me
Comment 7 Ryosuke Niwa 2010-08-09 16:50:57 PDT
(In reply to comment #6)
> (From update of attachment 63421 [details])
> Looks good, r=me

Thanks for the review again!
Comment 8 Ryosuke Niwa 2010-08-09 17:24:33 PDT
Committed r65019: <http://trac.webkit.org/changeset/65019>