Bug 9223 - Resize handles should be on the lower left corner for rtl
Summary: Resize handles should be on the lower left corner for rtl
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 420+
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 53913 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-06-01 17:17 PDT by Adele Peterson
Modified: 2023-05-07 09:44 PDT (History)
20 users (show)

See Also:


Attachments
Test case (505 bytes, text/html)
2009-01-07 14:11 PST, Jeremy Moskovich
no flags Details
A quick fix v1 (7.45 KB, patch)
2012-03-05 02:43 PST, Hironori Bono
no flags Details | Formatted Diff | Diff
A quick fix v2 (fixed the layout test) (7.44 KB, patch)
2012-03-06 04:15 PST, Hironori Bono
no flags Details | Formatted Diff | Diff
A quick fix v3 (Applied comments, for landing) (7.36 KB, patch)
2012-03-07 01:30 PST, Hironori Bono
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adele Peterson 2006-06-01 17:17:11 PDT
Consider adding the ability to resize the lower left corner for rtl textareas
Comment 1 Dave Hyatt 2006-06-02 18:11:33 PDT
Should only do this if we also move the scrollers.  Having the scrollers on the right but the resizer on the left would be pretty strange.
Comment 2 Jeremy Moskovich 2009-01-07 13:57:12 PST
http://code.google.com/p/chromium/issues/detail?id=6097
Comment 3 Jeremy Moskovich 2009-01-07 14:11:16 PST
Having the grabber on the right can lead to some pretty unintuitive behavior.

e.g.:
1. open the attached test case.
2. Hold down the mouse button over the grabber of the rtl textarea and then move the mouse a little to the right of the grabber graphic.
2. Continue holding down the mouse button and move the mouse up and down vertically.

What happens:
* The grabber doesn't "stay under the mouse" like it does in ltr textareas so the act of moving the grabber up and down vertically causes the textarea to continue to grow *horizontally* in an unbounded manner.
Comment 4 Jeremy Moskovich 2009-01-07 14:11:55 PST
Created attachment 26508 [details]
Test case
Comment 5 Yair Yogev 2009-05-14 03:55:57 PDT
this is not only a mac issue of course
Comment 6 Jeremy Moskovich 2011-02-07 11:46:59 PST
*** Bug 53913 has been marked as a duplicate of this bug. ***
Comment 7 Ryosuke Niwa 2012-02-23 11:52:29 PST
This bug is independent of scrollbars.
Comment 8 Ojan Vafai 2012-02-23 13:54:39 PST
(In reply to comment #7)
> This bug is independent of scrollbars.

I don't think it is. As per comment 1, which I think is correct. I think we should both move scrollbars and the resize handle. But we should *not* do one without the other.
Comment 9 Ryosuke Niwa 2012-02-23 14:01:58 PST
(In reply to comment #8)
> (In reply to comment #7)
> > This bug is independent of scrollbars.
> 
> I don't think it is. As per comment 1, which I think is correct. I think we should both move scrollbars and the resize handle. But we should *not* do one without the other.

That's a good point. This is a dup. of the bug 54623 then.
Comment 10 Hironori Bono 2012-02-29 21:14:22 PST
Greetings Jeremy,

Even though my change for Bug 54623 moves a resizer to the left side, I wonder if it totally satisfies your request. (It does not change the bitmap used for a resizer and it may look weird for you. Also, does this Bug expect that an RTL resizer increases its size when we drag it to the left side?)

Regards,

Hironori Bono
Comment 11 Ryosuke Niwa 2012-02-29 23:35:15 PST
(In reply to comment #10)
> Even though my change for Bug 54623 moves a resizer to the left side, I wonder if it totally satisfies your request. (It does not change the bitmap used for a resizer and it may look weird for you. Also, does this Bug expect that an RTL resizer increases its size when we drag it to the left side?)

We should definitely mirror the image. I think we want it to resize to the left ideally. But I guess that's possible only if the containing block is RTL?
Comment 12 Hironori Bono 2012-03-01 01:54:50 PST
Greetings Niwa-san,

Yes, they are possible. (We also need a Chromium change to add a bitmap resource, though.) I will upload a change and a resizer test when I land my change. (This was a reason why I did not add a resizer test to my change for Bug 54623.)

Regards,

Hironori Bono
Comment 13 Jeremy Moskovich 2012-03-01 02:47:17 PST
Bono-san: Can we mirror the resizer image using css?

IMHO you can fix the left drag behavior in a followup patch...
Comment 14 Yair Yogev 2012-03-01 03:18:34 PST
if left drag is only possible when the containing block is RTL,
would the re-sizer of an RTLed textarea work in a reasonable way if it's in an LTR block? (having the resize bitmap on the left and yet resize to the right)

Because if not (if it would be just like resizing a textarea in an RTLed block right now- awkward) than should the resizer really move with the scrollbar and not be influenced solely by the containing block?
Comment 15 Ryosuke Niwa 2012-03-01 11:58:46 PST
(In reply to comment #14)
> if left drag is only possible when the containing block is RTL,
> would the re-sizer of an RTLed textarea work in a reasonable way if it's in an LTR block? (having the resize bitmap on the left and yet resize to the right)
> 
> Because if not (if it would be just like resizing a textarea in an RTLed block right now- awkward) than should the resizer really move with the scrollbar and not be influenced solely by the containing block?

That would be weird though. I've started to think that maybe we can place scrollbar on the left for RTL textareas only if the containing block is also RTL.
Comment 16 Ryosuke Niwa 2012-03-01 12:01:34 PST
In fact, maybe scrollbar should be placed on either side based on the direction of the containing block.
Comment 17 Hironori Bono 2012-03-01 21:17:00 PST
Greetings Jeremy,

Thanks for your feedback.
In brief, we can mirror a resizer image as CSS does. Nevertheless, it means we need to mirror a resizer image every time when we render it. Even though I have not measured the speed of mirroring a resizer image, it may hurt the rendering speed. Would it be possible to give me your preference: using CSS or using a mirrored image?

Regards,

Hironori Bono

(In reply to comment #13)
> Bono-san: Can we mirror the resizer image using css?
Comment 18 Jeremy Moskovich 2012-03-02 04:59:45 PST
I'm not sure how much weight my opinion has since I'm not a reviewer and have little knowledge of the likely performance characteristics.

But IMHO we can try mirroring with css first and if we see a performance hit then a new resource can be added.

Does that sound OK to everyone?
Comment 19 Ojan Vafai 2012-03-02 10:24:54 PST
(In reply to comment #18)
> I'm not sure how much weight my opinion has since I'm not a reviewer and have little knowledge of the likely performance characteristics.
> 
> But IMHO we can try mirroring with css first and if we see a performance hit then a new resource can be added.
> 
> Does that sound OK to everyone?

Sounds fine to me.
Comment 20 Aharon (Vladimir) Lanin 2012-03-04 01:11:49 PST
Took a look at textarea resizing in Canary. Currently, resizing an RTL textarea is badly broken (besides the non-flipped image):
- Dragging the bottom left corner to the left does not do anything. It should widen the textarea by the distance from the mouse to the left corner.
- Dragging the bottom left corner to the right widens the textarea, crazily. It should not do anything.

I think that the most important test case is:

data:text/html,<html dir=rtl><textarea></textarea></html>

It should work as the exact mirror image of:

data:text/html,<html><textarea></textarea></html>

Please check out how it works in FF, which has it correct.

IMO, this is a severe regression and has to be fixed ASAP.

Now, regarding an RTL textarea in an LTR div, i.e.:

data:text/html,<html><textarea dir=rtl></textarea></html>

It has been suggested above that in this case the resizer should be on the bottom right and widen the textarea when dragged to the right. It has also been suggested that generally the resizer location and action should depend on the directionality of the textarea's containing block, not the directionality of the textarea itself. And that the resizer and the scrollbar should always be on the same side, so that in the case above the textarea's scrollbar should also be on the right.

Now, I agree that in the case above, the best UX would be to have the resizer on the bottom right, widening the textarea when dragged to the right. However, this has *nothing* to do with the directionality of either the textarea or its containing block. The exact same issue exists in the following three cases, *none* of which have any RTL in them:

data:text/html,<div style="text-align:right"><textarea></textarea> *** </div>

data:text/html,<div style="text-align:right"><div style="display:inline-block; text-align:left"><textarea></textarea> *** </div></div>

data:text/html,<div style="float:right"><textarea></textarea> *** </div>

If someone can figure how to handle these better than we do currently, that's fine. (BTW, please note that FF handles them more simply - and arguably better - than WebKit.) But it is not imperative to do so immediately. And the fix, although it will also improve the RTL textarea in LTR container (and LTR textarea in RTL container) case, has nothing to do with directionality. 

As for the resizer and the scrollbar having to be on the same side of the textarea, I completely disagree with that conclusion. Yes, ideally the resizer should be on the end side of the textarea, for the same reason that the scrollbar should be there. But having a reasonable UX for the resizer is much more important. If that means moving it to the start side of the textarea in certain cases, so be it. But I see absolutely no reason to move the scrollbar to the same side in those cases. Whatever we do, we should keep the scrollbar side interoperable with other browsers, which the recent fix to Bug 54623 finally achieved (for elements below body).
Comment 21 Hironori Bono 2012-03-05 02:43:20 PST
Created attachment 130086 [details]
A quick fix v1

Greetings,

Many thanks for your feedback.
I have quickly implemented a change that mirrors a resizer image and its behavior. (This change is not so tricky as the one for Bug 54623.)

Regards,

Hironori Bono
Comment 22 WebKit Review Bot 2012-03-05 03:33:23 PST
Comment on attachment 130086 [details]
A quick fix v1

Attachment 130086 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11803942

New failing tests:
platform/chromium/scrollbers/drag-rtl-resizer.html
Comment 23 Tony Chang 2012-03-05 11:50:50 PST
Comment on attachment 130086 [details]
A quick fix v1

This seems fine.  Is the test failing because RTL_SCROLLBAR isn't defined?  Please fix the test before landing.

Aharon makes a lot of good points in comment #20, but we should file a new bug for those issues.
Comment 24 Hironori Bono 2012-03-06 04:15:42 PST
Created attachment 130351 [details]
A quick fix v2 (fixed the layout test)

Greetings Tony,

Thanks for your review and comment.
Sorry, this test failure is totally my fault. The previous test does not work well on Chromium DumpRenderTree due to a variable-name conflict: this uses a variable 'top', which causes a conflict with 'window.top'. (I have tested my previous test only on Safari DumpRenderTree with WTF_USE_RTL_SCROLLBAR enabled.) I have updated the layout test. (It should work well with Chromium DumpRenderTree.)

Regards,

Hironori Bono

(In reply to comment #23)
> (From update of attachment 130086 [details])
> This seems fine.  Is the test failing because RTL_SCROLLBAR isn't defined?  Please fix the test before landing.
> 
> Aharon makes a lot of good points in comment #20, but we should file a new bug for those issues.
Comment 25 Tony Chang 2012-03-06 10:14:27 PST
Comment on attachment 130351 [details]
A quick fix v2 (fixed the layout test)

View in context: https://bugs.webkit.org/attachment.cgi?id=130351&action=review

> LayoutTests/platform/chromium/scrollbars/drag-rtl-resizer.html:13
> +<div id="console">
> +</div>

Nit: The console div now gets created lazily so you don't need this.

> LayoutTests/platform/chromium/scrollbars/drag-rtl-resizer.html:33
> +if (window.layoutTestController)
> +    layoutTestController.dumpAsText();

Nit: You can remove these, the test harness does this automatically.

> LayoutTests/platform/chromium/scrollbars/drag-rtl-resizer.html:46
> +var successfullyParsed = true;

Nit: You don't need this anymore.  It shouldn't change the output.
Comment 26 Hironori Bono 2012-03-07 01:30:13 PST
Created attachment 130567 [details]
A quick fix v3 (Applied comments, for landing)
Comment 27 Hironori Bono 2012-03-07 01:32:03 PST
Comment on attachment 130351 [details]
A quick fix v2 (fixed the layout test)

View in context: https://bugs.webkit.org/attachment.cgi?id=130351&action=review

>> LayoutTests/platform/chromium/scrollbars/drag-rtl-resizer.html:13
>> +</div>
> 
> Nit: The console div now gets created lazily so you don't need this.

Done. Thanks for point it out.

>> LayoutTests/platform/chromium/scrollbars/drag-rtl-resizer.html:33
>> +    layoutTestController.dumpAsText();
> 
> Nit: You can remove these, the test harness does this automatically.

Done. Thanks for point it out. (I did not notice it.)

>> LayoutTests/platform/chromium/scrollbars/drag-rtl-resizer.html:46
>> +var successfullyParsed = true;
> 
> Nit: You don't need this anymore.  It shouldn't change the output.

Done.
Comment 28 WebKit Review Bot 2012-03-07 11:08:18 PST
Comment on attachment 130567 [details]
A quick fix v3 (Applied comments, for landing)

Clearing flags on attachment: 130567

Committed r110073: <http://trac.webkit.org/changeset/110073>
Comment 29 WebKit Review Bot 2012-03-07 11:08:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 30 Aharon (Vladimir) Lanin 2012-03-08 14:03:40 PST
Looks good in Chrome Canary.
Comment 31 Jeremy Moskovich 2012-03-08 14:14:37 PST
Very nice work!!!  There is still a small visual glitch on the Mac - filed as bug 80640.
Comment 32 Alexey Proskuryakov 2012-03-08 21:20:50 PST
Why was this marked resolved? It's exceedingly obvious that this bug requests a Safari fix, not a Chrome one.
Comment 33 Hironori Bono 2012-03-08 21:40:38 PST
Greetings,

Sorry for closing this issue without fixing it on Safari. I made this change Chromium-only just because Dave wrote "I also don't believe we want this change on Mac OS X..." at <http://webkit.org/b/54623#c12>. Even though I may misunderstand his comment, I thought Safari did not need to move scrollbars or resizers. (This change moves scrollbars and resizers only when WTF_USE_RTL_SCROLLBAR is 1. This flag is set only on Chromium.) Regardless whether Safari needs to move scrollbars and resizers, we should have kept this issue open until Safari does move them as you noted.

Regards,

Hironori Bono

(In reply to comment #32)
> Why was this marked resolved? It's exceedingly obvious that this bug requests a Safari fix, not a Chrome one.
Comment 34 Eric Seidel (no email) 2012-03-27 12:51:25 PDT
Comment on attachment 130086 [details]
A quick fix v1

Cleared Tony Chang's review+ from obsolete attachment 130086 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 35 Eric Seidel (no email) 2012-03-27 12:51:30 PDT
Comment on attachment 130351 [details]
A quick fix v2 (fixed the layout test)

Cleared Tony Chang's review+ from obsolete attachment 130351 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 36 Ahmad Saleem 2023-05-07 00:01:54 PDT
Attached test case does not show any 'grabber' cursor on Safari Technology Preview 169 & WebKit ToT (263769@main) while it does show for Chrome Canary 115.
Comment 37 zalan 2023-05-07 09:44:08 PDT
"Resize handles should be on the lower left corner for rtl" <- Safari on macOS places resize control at the lower left for rtl text area.