Bug 92236

Summary: [Chromium][Win] <progress> animation on RTL direction is reverse
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Hajime Morrita
Reported 2012-07-25 03:49:06 PDT
This upstreams http://code.google.com/p/chromium/issues/detail?id=138247
Attachments
Patch (5.63 KB, patch)
2012-07-25 04:02 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (8.04 KB, patch)
2012-07-25 19:01 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch for landing (8.04 KB, patch)
2012-07-25 19:08 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-07-25 04:02:22 PDT
Created attachment 154319 [details] Patch
Kent Tamura
Comment 2 2012-07-25 04:40:34 PDT
Comment on attachment 154319 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154319&action=review > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:361 > + saveTransformIfNecessary(o, i, rect); > PlatformSupport::paintThemePart(i.context, PlatformSupport::PartProgressBar, getWebThemeState(this, o), rect, &extraParams); The function name 'saveTransformIfNecessary' isn't good for code readability. A reader can't understand what is done in saveTransformIfNecessary. Also, we had better use RAII idiom.
Hajime Morrita
Comment 3 2012-07-25 19:01:02 PDT
Created attachment 154518 [details] Patch
Hajime Morrita
Comment 4 2012-07-25 19:02:05 PDT
Thanks for the review, Kent-san, I updated the patch which employs a scoping class. Could you take another look?
Kent Tamura
Comment 5 2012-07-25 19:04:16 PDT
Comment on attachment 154518 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154518&action=review > Source/WebCore/rendering/RenderThemeChromiumSkia.h:166 > + ~DirectionFlippingScope(); > + private: nit: Need a blank line before private:.
Hajime Morrita
Comment 6 2012-07-25 19:08:24 PDT
Created attachment 154521 [details] Patch for landing
Hajime Morrita
Comment 7 2012-07-25 19:09:04 PDT
Thanks for the quick response! (In reply to comment #5) > (From update of attachment 154518 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154518&action=review > > > Source/WebCore/rendering/RenderThemeChromiumSkia.h:166 > > + ~DirectionFlippingScope(); > > + private: > > nit: Need a blank line before private:. Landing updated pach.
WebKit Review Bot
Comment 8 2012-07-25 20:27:13 PDT
Comment on attachment 154521 [details] Patch for landing Clearing flags on attachment: 154521 Committed r123706: <http://trac.webkit.org/changeset/123706>
WebKit Review Bot
Comment 9 2012-07-25 20:27:17 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.