Bug 80640 - RTL textarea resize handle looks bad on OS X
: RTL textarea resize handle looks bad on OS X
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2012-03-08 14:10 PST by
Modified: 2012-05-08 10:50 PST (History)


Attachments
Screenshot (22.15 KB, image/jpeg)
2012-03-08 14:11 PST, Jeremy Moskovich
no flags Details
testcase (505 bytes, text/html)
2012-03-08 14:12 PST, Jeremy Moskovich
no flags Details
Patch v1 (9.67 KB, patch)
2012-04-22 22:53 PST, Hironori Bono
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-03-08 14:10:13 PST
The fix for issue 9223 moved the resize handle to the left side for RTL textareas, in addition the resize handle image is mirrored.

On OS X the resize handle image for RTL texteareas overlaps the textarea border (see attached screenshot), this looks funny.

It needs to be inset by a pixel or 2 to match the LTR case.
------- Comment #1 From 2012-03-08 14:11:57 PST -------
Created an attachment (id=130901) [details]
Screenshot
------- Comment #2 From 2012-03-08 14:12:29 PST -------
Created an attachment (id=130902) [details]
testcase
------- Comment #3 From 2012-03-08 20:23:58 PST -------
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
------- Comment #4 From 2012-04-15 23:02:57 PST -------
Hi Bono-san, did you get a chance to look into this?
------- Comment #5 From 2012-04-15 23:17:00 PST -------
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?
------- Comment #6 From 2012-04-22 21:27:13 PST -------
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
------- Comment #7 From 2012-04-22 22:53:25 PST -------
Created an attachment (id=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
------- Comment #8 From 2012-04-23 19:11:12 PST -------
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
------- Comment #9 From 2012-04-24 00:21:54 PST -------
rtl-resizer-position-expected.png lgtm, thanks Bono-san!
------- Comment #10 From 2012-04-25 01:11:31 PST -------
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 #11 From 2012-04-27 10:46:47 PST -------
(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.
------- Comment #12 From 2012-05-07 21:04:11 PST -------
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] [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.
------- Comment #13 From 2012-05-08 10:02:17 PST -------
(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.
------- Comment #14 From 2012-05-08 10:04:31 PST -------
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 #15 From 2012-05-08 10:50:47 PST -------
(From update of attachment 138293 [details])
Clearing flags on attachment: 138293

Committed r116435: <http://trac.webkit.org/changeset/116435>
------- Comment #16 From 2012-05-08 10:50:53 PST -------
All reviewed patches have been landed.  Closing bug.