511180 - <resizer> should allow resizing of specific elements

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Neil Deakin]

Breaking this out of bug 167951.

The <resizer> currently resizes windows, and soon, the enclosing panel. It would be nice if it could instead resize some other element such as its parent. This would allow placing a resizer in a textarea so it resizes its parent textarea.

Currently, I'm thinking an attribute:

<resizer element="someid">

where a special value "_parent" means the parent element, and no value means the nearest enclosing popup/window.

Comment 1

[Neil Deakin]

Attachment: implement as described

Comment 2

[Neil Deakin]

Comment on attachment 395377 [details] [diff] [review]
implement as described

This implements this for elements as well as combines the patch and fixes from bug 511011

Comment 3

[[e-mail]]

Do you have a demo of this handy?

Should the element being resized be an ancestor of the resizer element?

What happens if you try to resize a panel by id (instead of it autodetecting)?

Comment 4

[Neil Deakin]

(In reply to comment #3)
> Do you have a demo of this handy?

No, but I can make a build.

> Should the element being resized be an ancestor of the resizer element?

Doesn't need to be. It would be useful for resizer handles for instance that wouldn't be on top of the element to resize.

> What happens if you try to resize a panel by id (instead of it autodetecting)?

I'll change this to just get the frame for to element and check if it is a menupopup as well.

Comment 5

[[e-mail]]

I meant a demo page/extension/whatever is necessary to see it in action.

Comment 6

[Neil Deakin]

Attachment: updates patch, also adds a resizer in popup test

Comment 7

[Neil Deakin]

Attachment: demo of resizers

Comment 8

[Neil Deakin]

You'll want to open it in a chrome window to test the popup resizing.

Comment 9

[[e-mail]]

(In reply to comment #8)
> You'll want to open it in a chrome window to test the popup resizing.
Not if I use DOM Inspector to set element="popup" on the resizer ;-)

Comment 10

[[e-mail]]

I mean element="panel" of course...

Comment 11

[Neil Deakin]

Attachment: move chrome shell check

This patch is the same as the last except that it moves the chrome shell check so that popups can be resized. Only window resizing is prevented.

Comment 12

[[e-mail]]

Comment on attachment 402355 [details] [diff] [review]
move chrome shell check

There are two issues I noticed while testing this.
The first is when the popup opens above the element. In a chrome frame, the resizer seems to be able to resize the popup based on where it opened (so seemingly detaching it from its menu). In a content frame, the resizer was unable to detach the popup from the menu, instead the popup resized in an amusing way, since the resizer did not track the mouse position.

The second issue is that my mouse-fu is very bad, and I often accidentally right-click while dragging. On something like a scrollbar, this just seems to cancel the drag. However on a resizer, at least with this code, this seems to leave the resizer in an indeterminate state whereby it wants to resize if you manage to keep the mouse over it. (I didn't think to test ordinary resizers.)

>+            // for native drags, don't set the fields below
>+            doDefault = PR_FALSE;
break here perhaps, to avoid testing doDefault again?

>+          // we're tracking.
Nit: either "We're tracking." or "we're tracking".

>+        // Don't allow resizing a window or a popup past the edge of the
>+        // screen, so adjust the rectangle to fit within the available screen
>+        // area.
Hmm, I think popups are normally allowed to overlap taskbars, aren't they?

>+          CallQueryInterface(treeOwner, aWindow);
Interestingly this gets the physical window, even when the resizer is in a chrome frame! I guess I should have added some flex to my test frame.

Comment 13

[[e-mail]]

(In reply to comment #12)
> The second issue is that my mouse-fu is very bad, and I often accidentally
> right-click while dragging. On something like a scrollbar, this just seems to
> cancel the drag. However on a resizer, at least with this code, this seems to
> leave the resizer in an indeterminate state whereby it wants to resize if you
> manage to keep the mouse over it. (I didn't think to test ordinary resizers.)
OK, so ordinary resizers seem to have this issue too.

Comment 14

[Neil Deakin]

Attachment: address comments, and add a check to ensure resizing doesn't work past the anchor point

Comment 15

[[e-mail]]

Something seems to be wrong with this patch. Everything resizes the window...

Comment 16

[Neil Deakin]

Works OK here on Windows and Mac. Problem with your build?

Comment 17

[[e-mail]]

(In reply to comment #16)
> Works OK here on Windows and Mac. Problem with your build?
Yes. Building and running the patched tree is always advisable!

(From update of attachment 406232 [details] [diff] [review])
I actually triggered the case of the popup opening above the element by the simple expedient of making the popup too large to open below the element; the updated patch does unfortunately not fixed this case. I did notice that the related case of using a bottomend resizer with a before_start panel is fixed, but I discovered an unusual result when attempting to use a before_start panel with a topend resizer: dragging the resizer down dragged the popup down with it, so that it ended up at the bottom of the screen!

>+        if (doDefault) {
>+          // we're tracking
doDefault is always true here.

>+        // Don't allow resizing a window or a popup past the edge of the
>+        // screen, so adjust the rectangle to fit within the available screen
>+        // area.
Which unfortunately exacerbates the popup resizing problem - using your test case, maximise the (chrome) window, then enlarge the upper button so that the panel opens near the bottom of the screen, then enlarge the panel right to the very bottom of the screen, and watch as it flips up out of the way! (With a large enough screen you can then grab the resizer again and repeat the effect.)

Comment 18

[Neil Deakin]

Attachment: updated patch

This patch fixes the issue with the popup flipping around. I also removed the checks for moving in content windows since the positioning should be working much better now such that popups won't extend outside the content area. 

I think I'll fix the popup shifting downwards issue separately as it has to do with the minimum size of the popup. It may need to wait for bug 357725.

Comment 19

[[e-mail]]

Comment on attachment 409951 [details] [diff] [review]
updated patch

Maybe when trying to shrink the height of a flipped/top popup you shouldn't move it if you are unable to resize it?

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

GetConstrainRect should be GetConstrainingRect or GetConstraintRect

+    if(mTrackingMouseMove && aEvent->eventStructType == NS_MOUSE_EVENT &&

if (

+    if(mTrackingMouseMove)

if (

Why do we need an "element" attribute? Why not just resize the nearest enclosing panel or window?

Comment 21

[Neil Deakin]

(In reply to comment #20)

> Why do we need an "element" attribute? Why not just resize the nearest
> enclosing panel or window?

The whole point of the bug is to allow adding a resizer within a multiline textbox/textarea.

Comment 22

[Neil Deakin]

http://hg.mozilla.org/mozilla-central/rev/4c8a5f1f1fef

Comment 23

[Eric Shepherd [:sheppy]]

Documented:

https://developer.mozilla.org/en/XUL/resizer

Comment 25

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336