Closed
Bug 555482
Opened 15 years ago
Closed 13 years ago
should be able to reset a resized textarea
Categories
(Core :: Layout: Form Controls, defect, P2)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla9
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: dietrich, Assigned: khuey)
References
(Depends on 2 open bugs)
Details
(Keywords: user-doc-needed)
Attachments
(1 file, 2 obsolete files)
13.88 KB,
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
||
We really need this. I keep running into this problem all the time.
blocking2.0: --- → ?
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?
Weird. In facebook's chat window, hitting escape resets the size of the textarea. I wonder how they do that?
Comment 4•15 years ago
|
||
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
Depends on: 553576
Comment 5•15 years ago
|
||
With a flush in between or something? That seems like a really unfortunate side-effect for a style change to have...
blocking2.0: ? → final+
Priority: -- → P2
Comment 7•14 years ago
|
||
(In reply to comment #6)
> I'm going to give this a try.
update?
Assignee | ||
Comment 8•14 years ago
|
||
Been busy, haven't had a chance to make much progress.
Comment 9•14 years ago
|
||
If that's the case, should this really be blocking? It's not vital to the release. The target milestone should be future.
Assignee | ||
Comment 10•14 years ago
|
||
I agree that this probably shouldn't block. Asking for re-triage.
blocking2.0: final+ → ?
Updated•14 years ago
|
blocking2.0: ? → -
Assignee | ||
Updated•14 years ago
|
Assignee: khuey → nobody
Assignee | ||
Comment 11•13 years ago
|
||
I finally got around to writing a patch for this. My layout-fu is pretty weak, so a thorough review would be appreciated.
Assignee | ||
Updated•13 years ago
|
Attachment #552926 -
Flags: review? → review?(enndeakin)
Assignee | ||
Comment 12•13 years ago
|
||
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•13 years ago
|
||
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.
Assignee | ||
Comment 14•13 years ago
|
||
(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•13 years ago
|
||
> 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.
Assignee | ||
Updated•13 years ago
|
Attachment #552926 -
Attachment is obsolete: true
Attachment #552926 -
Flags: review?(enndeakin)
Assignee | ||
Comment 16•13 years ago
|
||
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.
Attachment #553472 -
Flags: review?(enndeakin)
Assignee | ||
Comment 17•13 years ago
|
||
Enn points out on IRC that the previous test was bogus.
Attachment #553472 -
Attachment is obsolete: true
Attachment #553480 -
Flags: review?(enndeakin)
Attachment #553472 -
Flags: review?(enndeakin)
Assignee | ||
Comment 18•13 years ago
|
||
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•13 years ago
|
||
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.
Attachment #553480 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 20•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ab68ef485d77
Marking this in-testsuite+ ... but I'm not sure that the test is actually running anywhere ...
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Assignee | ||
Updated•13 years ago
|
Keywords: user-doc-needed
Comment 21•13 years ago
|
||
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.
Assignee | ||
Comment 22•13 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•