Bug 80640 – RTL textarea resize handle looks bad on OS X

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Jeremy Moskovich]

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.

[Jeremy Moskovich]

Created attachment 130901 [details]
Screenshot

[Jeremy Moskovich]

Created attachment 130902 [details]
testcase

[Hironori Bono]

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

[Jeremy Moskovich]

Hi Bono-san, did you get a chance to look into this?

[Hironori Bono]

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?

[Hironori Bono]

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

[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

[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

[Jeremy Moskovich]

rtl-resizer-position-expected.png lgtm, thanks Bono-san!

[Hironori Bono]

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!

[Tony Chang]

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.

[Hironori Bono]

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.

[Tony Chang]

(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.

[Tony Chang]

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.

[WebKit Review Bot]

Comment on attachment 138293 [details]
Patch v1

Clearing flags on attachment: 138293

Committed r116435: <http://trac.webkit.org/changeset/116435>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.