[patch][cla] ResizeHandle maxSize and fixedAspect errors, _checkConstraints() bug?

[gjf]

I'm on 1.3.2. Firefox 3.5.3. windoze. And am reporting two related issues in ResizeHandle?. Bug1 is related to fixedAspect; when sizing the resize box sometimes changes from nxm to mxn. Bug2 is related to maxSize constraint: the constraint works fine in one direction (width is properly constrained in my case) but is not constrained in the y direction.

I don't have the proper patent releases etc so I won't try to work up code fix this but the following notes may help:

I think there are two errors in the _checkConstraints() method.

Bug1 occurs when the new aspect ratio would be wide (w > h) and the fixed aspect is tall (h > w). The code produces a wide result where a tall one is needed.

Bug2 occurs when both there is both a fixeAspect and maxSize constraints. Because the maxSize is checked first, the fixedAspect correction can increase the height [width] beyond the max.

Hope this is helpful. Let me know if you have any questions. Cheers, Gordon

[dante]

the constrain options are "highly experimental" (as per the tests?) but this is important (just can't get to it before 1.4)

[Johnathon Harris]

We have addressed the problems with resizing non square fixed aspect ratios in dojo-1.3.2, patch is attached.

Note we also modified the arbitrary minSize of 100x100. The code now defaults to a minSize of the target node size.

The patch includes some updates to the test case file showing the new behaviour.

[Johnathon Harris]

Support non square fixed aspect ratio resizing. Updated: better handling of non active resize box.

[dante]

nice. Do you have a CLA on file?

[Johnathon Harris]

Hi Dante, I do not not have a CLA on file but am aware of the need for one- these modifications were performed at work and so we are reviewing the corporate CLA. Should get sorted this week.

[Johnathon Harris]

Sorry, didn't intend the double negative in the previous comment. Will update when a CLA is filed.

[dante]

okay, great. Going to punt this to 1.6 for now, 1.5beta will be very soon. just ping when the CLA/CCLA is in place. Thanks!

[Johnathon Harris]

Hi Dante, we now have a corporate CLA on file for Certus Technology. Is this patch still ok for inclusion or does it need updating given that a few months have passed?

[dante]

it's likely fine to merge as is, nothing changed in resizeHandle since afaik. Will apply locally and test. If nothing crops up, I'll just commit.

[Colin Snover]

1.8 has been tagged; moving all outstanding tickets to next major release milestone.

[dylan]

I took a quick look at this one. Parts of it will apply cleanly, but other parts contain logic that's changed a fair amount.

I don't want to break this widget, but if you can get me a patch against 1.9 within the next week, I'll review and get it in for 1.9.

[trac-o-bot]

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!