Opened 10 years ago

Closed 6 years ago

#10146 closed defect (invalid)

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

Reported by: gjf Owned by: gjf
Priority: high Milestone: 1.9
Component: Dojox Version: 1.3.2
Keywords: Cc:
Blocked By: Blocking:

Description

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

Attachments (1)

dojo-1.3.2_ResizeHandle_bug10146.patch (5.8 KB) - added by Johnathon Harris 9 years ago.
Support non square fixed aspect ratio resizing. Updated: better handling of non active resize box.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by Adam Peller

Owner: changed from Adam Peller to dante

comment:2 Changed 9 years ago by dante

Milestone: tbd1.5
Status: newassigned

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

comment:3 Changed 9 years ago by 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.

Changed 9 years ago by Johnathon Harris

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

comment:4 Changed 9 years ago by dante

nice. Do you have a CLA on file?

comment:5 Changed 9 years ago by 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.

comment:6 Changed 9 years ago by Johnathon Harris

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

comment:7 Changed 9 years ago by dante

Milestone: 1.51.6

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!

comment:8 Changed 9 years ago by 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?

comment:9 Changed 9 years ago by 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.

comment:10 Changed 8 years ago by dante

Milestone: 1.61.7

comment:11 Changed 7 years ago by Colin Snover

Milestone: 1.82.0

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

comment:12 Changed 6 years ago by dylan

Milestone: 2.01.9
Owner: changed from dante to dylan
Summary: ResizeHandle maxSize and fixedAspect errors, _checkConstraints() bug?[patch][cla] ResizeHandle maxSize and fixedAspect errors, _checkConstraints() bug?

comment:13 Changed 6 years ago by dylan

Owner: changed from dylan to gjf
Status: assignedpending

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.

comment:14 Changed 6 years ago by trac-o-bot

Resolution: invalid
Status: pendingclosed

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!

Note: See TracTickets for help on using tickets.