Bug 43465

Summary: fontColorChangesComputedStyle, fontSizeChangesComputedStyle, and fontFaceChangesComputedStyle should be removed
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Enhancement CC: bdakin, enrica, justin.garcia
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch justin.garcia: review+

Ryosuke Niwa
Reported 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.
Attachments
Patch (4.20 KB, patch)
2010-08-03 22:10 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Patch (9.22 KB, patch)
2010-08-03 23:52 PDT, Ryosuke Niwa 		justin.garcia: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2010-08-03 22:10:22 PDT
Created attachment 63415 [details] Patch
Justin Garcia
Comment 2 2010-08-03 22:22:07 PDT
Comment on attachment 63415 [details] Patch r=me assuming you verified that that bug that Beth mentioned didn't regress.
Justin Garcia
Comment 3 2010-08-03 22:23:15 PDT
Ah, I just read your email, thanks Ryosuke!
Ryosuke Niwa
Comment 4 2010-08-03 23:52:20 PDT
Created attachment 63421 [details] Patch
Ryosuke Niwa
Comment 5 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 :(
Justin Garcia
Comment 6 2010-08-09 16:35:58 PDT
Comment on attachment 63421 [details] Patch Looks good, r=me
Ryosuke Niwa
Comment 7 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!
Ryosuke Niwa
Comment 8 2010-08-09 17:24:33 PDT
Committed r65019: <http://trac.webkit.org/changeset/65019>
Note You need to log in before you can comment on or make changes to this bug.