555482 - should be able to reset a resized textarea

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Dietrich Ayala (:dietrich)]

after using resizable textareas for a little while now, one thing i keep finding myself doing is trying to reset them back to normal size when finished editing the content.

it would be great if i could double-click on the resizer to reset the element back to it's default size.

another option could be to reset when the textarea is no longer focused.

a cool interaction would be to persist the user-set size somewhere, and apply it when focused and reset on unfocus.

Comment 1

[Boris Zbarsky [:bzbarsky]]

We really need this.  I keep running into this problem all the time.

Comment 2

[d]

Oh, I suggested the same thing earlier today in bug 553576.

"One thing that that struck me is that having a function to
automatically switch back to the original size would be useful, as it might
break the layout if it's resized in any way (and finding the original size
manually, might be hard). We have a few options if we want to do that, that I
can think of:

1. As soon as the resizer is used to change the size, a new option appears in
the right-click menu of the form, or perhaps only the resizer itself.
2. Doubleclicking the resizer-grip restores the original size.

Out of these two, I prefer the second one, if only one had to be chosen, but a
right-click menu option would be good to complete it."

Anything good for this bug in there?

Comment 3

[u88484]

Weird.  In facebook's chat window, hitting escape resets the size of the textarea.  I wonder how they do that?

Comment 4

[(no longer active)]

If the resize factor resetting behavior is implemented as part of bug 553576, this functionality can be implemented like below:

  textarea.style.resize = "none"; // to reset the textarea to its original size
  textarea.style.resize = "both"; // to make it resizable again

Comment 5

[Boris Zbarsky [:bzbarsky]]

With a flush in between or something?  That seems like a really unfortunate side-effect for a style change to have...

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'm going to give this a try.

Comment 7

[imradyurrad]

(In reply to comment #6)
> I'm going to give this a try.

update?

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Been busy, haven't had a chance to make much progress.

Comment 9

[imradyurrad]

If that's the case, should this really be blocking? It's not vital to the release. The target milestone should be future.

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I agree that this probably shouldn't block.  Asking for re-triage.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

I finally got around to writing a patch for this.  My layout-fu is pretty weak, so a thorough review would be appreciated.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Note that the attached patch allows resetting all resizers.  We might want to restrict that to just resizers that are attached to textareas ...

Comment 13

[Neil Deakin]

Comment on attachment 552926 [details] [diff] [review]
Patch

>+  // for XUL elements, just set the width and height attributes. For
>+  // other elements, set style.width and style.height
>+  if (aContent->IsXUL()) {
>+    if (aOriginalSizeInfo) {
>+      aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::width,
>+                        aOriginalSizeInfo->width);
>+      aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::height,
>+                        aOriginalSizeInfo->height);
>+    }

This will only work if the width/height was already present. It likely won't be for the original size. Same with the css inline style check, which will only work if style="width: ..." was explicitly set.

You may want to just get the size as per the code that sets mMouseDownRect.

>+      if (aDirection.mHorizontal) {
>+        nsString widthstr(aSizeInfo.width);

Generally, we would use nsAutoString here, and with height.

Please add a test as well, either to window_resizer_element.xul or in the same directory.

I think it might be necessary to include the screen constraint check for menupopup resizers, in case the window is now on a different sized screen. This will become more important with bug 357725. Or, you can just limit this patch to not apply to menupopup resizers.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Neil Deakin from comment #13)
> Comment on attachment 552926 [details] [diff] [review]
> Patch
> 
> >+  // for XUL elements, just set the width and height attributes. For
> >+  // other elements, set style.width and style.height
> >+  if (aContent->IsXUL()) {
> >+    if (aOriginalSizeInfo) {
> >+      aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::width,
> >+                        aOriginalSizeInfo->width);
> >+      aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::height,
> >+                        aOriginalSizeInfo->height);
> >+    }
> 
> This will only work if the width/height was already present. It likely won't
> be for the original size. Same with the css inline style check, which will
> only work if style="width: ..." was explicitly set.

I didn't test with XUL, but with an <html:textarea> this works fine.  We set the width/height properties to an empty string which unsets them and allows the element to resize back to whatever it was originally.

> You may want to just get the size as per the code that sets mMouseDownRect.

I don't understand this.  Do you mean in the same place or through the same method?

> >+      if (aDirection.mHorizontal) {
> >+        nsString widthstr(aSizeInfo.width);
> 
> Generally, we would use nsAutoString here, and with height.

Done.

> Please add a test as well, either to window_resizer_element.xul or in the
> same directory.

Yep.

> I think it might be necessary to include the screen constraint check for
> menupopup resizers, in case the window is now on a different sized screen.
> This will become more important with bug 357725. Or, you can just limit this
> patch to not apply to menupopup resizers.

Yeah, I think we should just exclude menupopup resizers here.

Comment 15

[Neil Deakin]

> I didn't test with XUL, but with an <html:textarea> this works fine.  We set
> the width/height properties to an empty string which unsets them and allows
> the element to resize back to whatever it was originally.

OK, that seems reasonable. The test should check for this case then.


> I don't understand this.  Do you mean in the same place or through the same method?

This isn't relevant any more given the previous.

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch (w/comments addressed)

Comments addressed.  I'm throwing this at try to run the test since it only runs on Mac.  :-/  I've verified manually that xul resizers in test_resizer.xul reset if you double click them.

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Enn points out on IRC that the previous test was bogus.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I threw this at try to run test_resizer.xul on Mac, but that test doesn't appear to actually run any checks, even without my patch ...

Comment 19

[Neil Deakin]

Comment on attachment 553480 [details] [diff] [review]
Patch

>       var newrect = document.getElementById(resizerid + "-container").getBoundingClientRect();
>       is(Math.round(newrect.width), Math.round(expectedWidth), "resize element " + resizerid +
>          " " + testid + " width moving " + mouseX + "," + mouseY + ",,," + hResize);
>       is(Math.round(newrect.height), Math.round(expectedHeight), "resize element " + resizerid +
>          " " + testid + " height moving " + mouseX + "," + mouseY);

...

>+      is(Math.round(newrect.width), Math.round(rect.width), "resize element " + resizerid +
>+         " " + testid + " width moving " + mouseX + "," + mouseY + ",,," + hResize);
>+      is(Math.round(newrect.height), Math.round(rect.height), "resize element " + resizerid +
>+         " " + testid + " height moving " + mouseX + "," + mouseY);

Also update the test descriptive message so it's different that the one above.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/ab68ef485d77

Marking this in-testsuite+ ... but I'm not sure that the test is actually running anywhere ...

Comment 21

[Asa Dotzler [:asa]]

Can someone involved here describe precisely what has been implemented? Is this a user feature or a web developer feature? How does one take advantage of this feature? Thanks.

Comment 22

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Asa Dotzler [:asa] from comment #21)
> Can someone involved here describe precisely what has been implemented? Is
> this a user feature or a web developer feature? How does one take advantage
> of this feature? Thanks.

If you resize a textarea (or anything else with a xul resizer, except menupopups) and then double click on the resizer it will reset to the original dimensions.  This is a user feature.