| Summary: | RTL textarea resize handle looks bad on OS X | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jeremy Moskovich <playmobil> | ||||||||
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | aharon, hbono, tony, webkit.review.bot, xji | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Jeremy Moskovich
2012-03-08 14:10:13 PST
Created attachment 130901 [details]
Screenshot
Created attachment 130902 [details]
testcase
Greetings Jeremy, Thanks for your bug report. Somehow, this issue happens only on Mac Chrome and Linux Chrome, i.e. it does not happen on Windows Chrome. (When I defined WTF_USE_RTL_SCROLLBAR on Safari, this issue also happens on Mac Safari.) I will investigate more deeply to find why my r110073 causes such platform differences. Regards, Hironori Bono Hi Bono-san, did you get a chance to look into this? Greetings Jeremy, Unfortunately, not so much. As written in my previous comment, this issue does not happen on Windows and I assume this issue is caused by the platform-specific code of WebKit. (I would note my r110073 does not have any platform-specific code.) But, I do not have so much knowledge about it and I have a problem understanding it. Regards, Hironori Bono (In reply to comment #4) > Hi Bono-san, did you get a chance to look into this? Greetings Jeremy, I have looked at this issue today and noticed it was caused by a bonehead mistake of my r110073 <http://trac.webkit.org/changeset/110073>. (Its translation formula is wrong and it works only on some special situations.) I'm writing a fix for this issue now. Regards, Hironori Bono Created attachment 138293 [details]
Patch v1
Greetings,
I have quickly written a fix and its layout test.
Unfortunately, this test is a pixel test and I'm not sure if it works on all platforms. (This test finishes without errors when I tested it on my Win7 PC, Mac, and a Linux PC, respectively.) I would like to set r? to run it on EWS bots and see its results.
Regards,
Hironori Bono
Greetings Jeremy, It seems my layout test works well on EWS bots. Would it be possible to check if the attached image is your expected result? Regards, Hironori Bono rtl-resizer-position-expected.png lgtm, thanks Bono-san! Greetings Tony, Would it be possible to review this quick fix? Regards, Hironori Bono (In reply to comment #9) > rtl-resizer-position-expected.png lgtm, thanks Bono-san! Comment on attachment 138293 [details]
Patch v1
Why is the layout test for Chromium only? Is it because this feature is currently only enabled on Chromium? It might be better to move tests for left side scrollbars and resizers into a subdir and skip that dir on platforms that don't enable this feature. That could be done in a follow up patch.
Greetings Tony, Thanks for your review and comment. Yes, I made this test Chromium-only one because this feature was enabled only on Chromium. Since I have landed another Chromium-only test (r110073) to "LayoutTests/platform/chromium", I would like to create another change that moves all of them to "LayoutTests/scrollbars/rtl". Is it OK for you? Regards, Hironori Bono (In reply to comment #11) > (From update of attachment 138293 [details]) > Why is the layout test for Chromium only? Is it because this feature is currently only enabled on Chromium? It might be better to move tests for left side scrollbars and resizers into a subdir and skip that dir on platforms that don't enable this feature. That could be done in a follow up patch. (In reply to comment #12) > Greetings Tony, > > Thanks for your review and comment. > Yes, I made this test Chromium-only one because this feature was enabled only on Chromium. Since I have landed another Chromium-only test (r110073) to "LayoutTests/platform/chromium", I would like to create another change that moves all of them to "LayoutTests/scrollbars/rtl". Is it OK for you? A separate change to move the tests is fine with me. Keeping the tests out of platform/chromium makes it easier for other ports that want to enable the feature. Also, I wonder how hard it would be to make this a ref test. You would probably have to include a copy of the reversed gripper with the test. Comment on attachment 138293 [details] Patch v1 Clearing flags on attachment: 138293 Committed r116435: <http://trac.webkit.org/changeset/116435> All reviewed patches have been landed. Closing bug. |