Opened 11 years ago
Closed 8 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)
Change History (15)
comment:1 Changed 11 years ago by
Owner: | changed from Adam Peller to dante |
---|
comment:2 Changed 11 years ago by
Milestone: | tbd → 1.5 |
---|---|
Status: | new → assigned |
comment:3 Changed 11 years ago by
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 11 years ago by
Attachment: | dojo-1.3.2_ResizeHandle_bug10146.patch added |
---|
Support non square fixed aspect ratio resizing. Updated: better handling of non active resize box.
comment:5 Changed 11 years ago by
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 11 years ago by
Sorry, didn't intend the double negative in the previous comment. Will update when a CLA is filed.
comment:7 Changed 11 years ago by
Milestone: | 1.5 → 1.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 10 years ago by
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 10 years ago by
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 10 years ago by
Milestone: | 1.6 → 1.7 |
---|
comment:11 Changed 8 years ago by
Milestone: | 1.8 → 2.0 |
---|
1.8 has been tagged; moving all outstanding tickets to next major release milestone.
comment:12 Changed 8 years ago by
Milestone: | 2.0 → 1.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 8 years ago by
Owner: | changed from dylan to gjf |
---|---|
Status: | assigned → pending |
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 8 years ago by
Resolution: | → invalid |
---|---|
Status: | pending → closed |
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!
the constrain options are "highly experimental" (as per the tests?) but this is important (just can't get to it before 1.4)