Bug 9223

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.

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.

Created attachment 26508 [details] Test case

this is not only a mac issue of course

*** Bug 53913 has been marked as a duplicate of this bug. ***

This bug is independent of scrollbars.

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

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

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

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

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

Bono-san: Can we mirror the resizer image using css? IMHO you can fix the left drag behavior in a followup patch...

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?

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

In fact, maybe scrollbar should be placed on either side based on the direction of the containing block.

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?

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?

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

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

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

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

Created attachment 130567 [details] A quick fix v3 (Applied comments, for landing)

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

All reviewed patches have been landed. Closing bug.

Looks good in Chrome Canary.

Very nice work!!! There is still a small visual glitch on the Mac - filed as bug 80640.

Why was this marked resolved? It's exceedingly obvious that this bug requests a Safari fix, not a Chrome one.

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

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.

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