Bug 72948 – resize:both does not respect min-width and max-width

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: CSS

[Felix Gnass]

For resizable elements (style="resize:both") WebKit currently only takes max-{width,height} into account but ignores the min-{width,height} properties. Instead the minimum dimensions are constrained by the initial size.

I've attached a simple example to demonstrate the behavior.
You may also view it online at http://jsfiddle.net/fgnass/xXwQV/

[Felix Gnass]

Created attachment 116328 [details]
A simple test-case

[David Elahee]

Hi, this bug was reported a year ago, on chrome, and is stil present on safari. We believe it is a really nice feature, and would be rather pleased to see the issue adressed. We use it mainly do wo windows alike widgeting.

http://code.google.com/p/chromium/issues/detail?can=2&start=0&num=100&q=min-height&colspec=ID%20Pri%20Mstone%20ReleaseBlock%20Area%20Feature%20Status%20Owner%20Summary&groupby=&sort=&id=72841

Thanks a lot.

*** Bug 124457 has been marked as a duplicate of this bug. ***

(In reply to comment #3)
> *** Bug 124457 has been marked as a duplicate of this bug. ***

Hi Darin, Simon. Wanna know your thoughts?

Spec says "The user agent may restrict the resizing range to something suitable, such as between the original formatted size of the element, and large enough to encompass all the element's contents."

I feel incase min-width/min-height is specified then it should be resizable to that value.

[Andre Schmidt]

jup, spec doesn't seem to define how to behave when min-/max- is set http://dev.w3.org/csswg/css-ui/#resize but my logic would be something like, allow resize down/up to min/max if set, otherwise till 0/infinity. my current use case would also be window like behaviour (and then thanks to box model, a step sequencer automatically splits to multiple rows inside, yay!).

[Simon Fraser (smfr)]

Respecting min/max sounds reasonable.

(In reply to comment #6)
> Respecting min/max sounds reasonable.

Thanks Simon and Andre for your views.
Firefox I have observed that incase min-width/height is not present or is zero it still keeps the min to 16*16. Only if min-width/height is greater then 16*16 it respects that.

Created attachment 221727 [details]
Patch

(In reply to comment #8)
> Created an attachment (id=221727) [details]
> Patch

Please review. Thanks.

[Simon Fraser (smfr)]

Comment on attachment 221727 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=221727&action=review

> Source/WebCore/rendering/RenderLayer.cpp:2629
> +    LayoutUnit minWidth = renderer->style().minWidth().value();
> +    LayoutUnit minHeight = renderer->style().minHeight().value();

Does this work if min-width is a % or auto?

Also, would be good to have a testcase that tests the interaction with zoom.

Created attachment 221838 [details]
Patch

(In reply to comment #11)
> Created an attachment (id=221838) [details]
> Patch

Thanks Simon for the review. Have made changes for percentage and added zoom test case.

For auto the min-width/min-height becomes zero.

[Simon Fraser (smfr)]

Comment on attachment 221838 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=221838&action=review

> Source/WebCore/rendering/RenderLayer.cpp:2633
> +    if (renderer->style().minWidth().isPercent())
> +        minWidth = renderer->style().minWidth().percent() / 100 * renderer->parent()->absoluteBoundingBoxRect().width();
> +    if (renderer->style().minHeight().isPercent())
> +        minHeight = renderer->style().minHeight().percent() / 100 * renderer->parent()->absoluteBoundingBoxRect().height();

This is the wrong way to resolve percentages etc. You should look at the code that use min/maxWidth to do layout.

Created attachment 221962 [details]
Patch

(In reply to comment #14)
> Created an attachment (id=221962) [details]
> Patch

Hi Simon. Can you please have a look. Thanks .

Created attachment 222410 [details]
Patch

(In reply to comment #16)
> Created an attachment (id=222410) [details]
> Patch

Hi Simon. Can you please review? Thanks

(In reply to comment #17)
> (In reply to comment #16)
> > Created an attachment (id=222410) [details] [details]
> > Patch
> 
> Hi Simon. Can you please review? Thanks

Can someone please review this ? Thanks.

[Andreas Kling]

CC'ing rendering & layout folks.

@jonlee: Who is a good person to review this?

Created attachment 224164 [details]
Patch

Created attachment 224483 [details]
Patch

(In reply to comment #21)
> Created an attachment (id=224483) [details]
> Patch

Test cases had inconsistent indentation. Rectified.

[Simon Fraser (smfr)]

Comment on attachment 224483 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=224483&action=review

> Source/WebCore/rendering/RenderLayer.cpp:2541
>      element->setMinimumSizeForResizing(minimumSize);

Why does it make sense to set the minimum sizes on the elements during resizing, and not at layout time?

Why is minimumSizeForResizing on Element at all? It seems more like a rendering thing.

(In reply to comment #23)
> (From update of attachment 224483 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=224483&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:2541
> >      element->setMinimumSizeForResizing(minimumSize);
> 
> Why does it make sense to set the minimum sizes on the elements during resizing, and not at layout time?
> 
> Why is minimumSizeForResizing on Element at all? It seems more like a rendering thing.

This part of the code was already there before my patch.

[zalan]

(In reply to comment #24)
> (In reply to comment #23)
> > (From update of attachment 224483 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=224483&action=review
> > 
> > > Source/WebCore/rendering/RenderLayer.cpp:2541
> > >      element->setMinimumSizeForResizing(minimumSize);
> > 
> > Why does it make sense to set the minimum sizes on the elements during resizing, and not at layout time?
> > 
> > Why is minimumSizeForResizing on Element at all? It seems more like a rendering thing.
> 
> This part of the code was already there before my patch.
While that's indeed the case, I think figuring out whether setting the minimum size is the right thing to do here helps finding the proper fix to this problem.

So for now we can remove the element related calls in resize function such as minimumSizeForResizing() and setMinimumSizeForResizing and instead set the minimumHeight and minimumWidth while layout i.e in RenderBox and use those variables values while resizing.

Later remove the resize code from Element. Please suggest?

http://trac.webkit.org/changeset/21184/trunk/WebCore/dom/Element.cpp has the Element resize related code.

[Simon Fraser (smfr)]

(In reply to comment #26)
> So for now we can remove the element related calls in resize function such as minimumSizeForResizing() and setMinimumSizeForResizing and instead set the minimumHeight and minimumWidth while layout i.e in RenderBox and use those variables values while resizing.
> 
> Later remove the resize code from Element. Please suggest?
> 
> http://trac.webkit.org/changeset/21184/trunk/WebCore/dom/Element.cpp has the Element resize related code.

That sounds reasonable. Please don't bloat RenderBox with new member variables thought.

[zalan]

(In reply to comment #27)
> (In reply to comment #26)
> > So for now we can remove the element related calls in resize function such as minimumSizeForResizing() and setMinimumSizeForResizing and instead set the minimumHeight and minimumWidth while layout i.e in RenderBox and use those variables values while resizing.
> > 
> > Later remove the resize code from Element. Please suggest?
> > 
> > http://trac.webkit.org/changeset/21184/trunk/WebCore/dom/Element.cpp has the Element resize related code.
> 
> That sounds reasonable. Please don't bloat RenderBox with new member variables thought.
and also make sure that you are not setting the min/max resize value on every element as that would end up creating extra raredata instances that you'd normally never use.

Created attachment 225744 [details]
Patch

(In reply to comment #29)
> Created an attachment (id=225744) [details]
> Patch

Thanks Simon and Zalan for the reviews. Please revies this patch.
As I observed that while resizing for max width/height we just increase the width/height of the content by +ve offset and since content cannot go beyond the max width/height it works. So done similiar changes for min with/height so that we decrease the width/height by -ve offset.

[zalan]

Comment on attachment 225744 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=225744&action=review

> Source/WebCore/rendering/RenderLayer.cpp:-2580
> -    element->setMinimumSizeForResizing(minimumSize);

You should also remove the minimumSizeForResizing/setMinimumSizeForResizing functions as no one else uses them.

> Source/WebCore/rendering/RenderLayer.cpp:2586
> +    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(newOffset) - currentSize;

I don't really see the logic in this change.  What's correlation between the element's (new)width and the cursor's distance from the resize corner with regard to the minimum size?

(In reply to comment #31)
> (From update of attachment 225744 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=225744&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:-2580
> > -    element->setMinimumSizeForResizing(minimumSize);
> 
> You should also remove the minimumSizeForResizing/setMinimumSizeForResizing functions as no one else uses them.

I thought this we would do in another patch so that its not confusing.

> 
> > Source/WebCore/rendering/RenderLayer.cpp:2586
> > +    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(newOffset) - currentSize;
> 
> I don't really see the logic in this change.  What's correlation between the element's (new)width and the cursor's distance from the resize corner with regard to the minimum size?

The logic for resizing as what I have understood is 
1)Find the offset (+ve or -ve) based on the mouse move events and the current size/location of the element which is done in offsetFromResizeCorner function. 
2)Set the CSSPropertyWidth or CSSPropertyHeight to the current size + offset. 
3)Then a document.updateLayout(); is done which takes care of not expanding the element more than its max width/height even if the offset is more than the max width/height nor shrinking it beyond its min width/height even if offset is less than min width/height. 
4)Once it has increased or decreased, the current size becomes the previous size + offset.
So newOffset is that value by which we need to increase or decrease it so I made the changes accordingly.

I observed one more thing that instead of using 
LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(minimumSize) - currentSize;
if we use
LayoutSize difference = newOffset; its still working.

[zalan]

(In reply to comment #32)
> (In reply to comment #31)
> > (From update of attachment 225744 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=225744&action=review
> > 
> > > Source/WebCore/rendering/RenderLayer.cpp:-2580
> > > -    element->setMinimumSizeForResizing(minimumSize);
> > 
> > You should also remove the minimumSizeForResizing/setMinimumSizeForResizing functions as no one else uses them.
> 
> I thought this we would do in another patch so that its not confusing.
> 
> > 
> > > Source/WebCore/rendering/RenderLayer.cpp:2586
> > > +    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(newOffset) - currentSize;
> > 
> > I don't really see the logic in this change.  What's correlation between the element's (new)width and the cursor's distance from the resize corner with regard to the minimum size?
> 
> The logic for resizing as what I have understood is 
> 1)Find the offset (+ve or -ve) based on the mouse move events and the current size/location of the element which is done in offsetFromResizeCorner function. 
> 2)Set the CSSPropertyWidth or CSSPropertyHeight to the current size + offset. 
> 3)Then a document.updateLayout(); is done which takes care of not expanding the element more than its max width/height even if the offset is more than the max width/height nor shrinking it beyond its min width/height even if offset is less than min width/height. 
> 4)Once it has increased or decreased, the current size becomes the previous size + offset.
> So newOffset is that value by which we need to increase or decrease it so I made the changes accordingly.
> 
> I observed one more thing that instead of using 
> LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(minimumSize) - currentSize;
> if we use
> LayoutSize difference = newOffset; its still working.
Well, that was my point. I think the difference is the new offset.

Created attachment 226483 [details]
Patch

> Well, that was my point. I think the difference is the new offset.

Agree and the function offsetFromResizeCorner calculates the newOffset based on the mouse events converted to contents and then taking the difference with respect to the current content position.

[zalan]

Looks good. Please add a follow up patch (separate bug) to get the *minimumSizeForResizing() functions removed.

(In reply to comment #36)
> Looks good. Please add a follow up patch (separate bug) to get the *minimumSizeForResizing() functions removed.

Thanks for the review Zalan.Will do that.

[Simon Fraser (smfr)]

Comment on attachment 226483 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=226483&action=review

> Source/WebCore/rendering/RenderLayer.cpp:-2589
> -    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(minimumSize) - currentSize;
> -

adjustedOldOffset is unused after this change, so you should remove it too.

Created attachment 226575 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 226575 [details]
Patch

Rejecting attachment 226575 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
 'oldOffset' [-Werror,-Wunused-parameter]
void RenderLayer::resize(const PlatformMouseEvent& evt, const LayoutSize& oldOffset)
                                                                          ^
1 error generated.

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/RenderLayer.o rendering/RenderLayer.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: http://webkit-queues.appspot.com/results/4961400896094208

Created attachment 226650 [details]
Patch

(In reply to comment #41)
> Created an attachment (id=226650) [details]
> Patch

Added new patch to resolve the mac error.

[Build Bot]

Comment on attachment 226650 [details]
Patch

Attachment 226650 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6050832235626496

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html
fast/css/resize-single-axis.html
fast/css/resize-corner-tracking-transformed.html
fast/overflow/hit-test-overflow-controls.html
fast/css/resize-corner-tracking.html

[Build Bot]

Created attachment 226656 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 226650 [details]
Patch

Attachment 226650 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4975660724387840

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html
fast/css/resize-single-axis.html
fast/css/resize-corner-tracking-transformed.html
fast/overflow/hit-test-overflow-controls.html
fast/css/resize-corner-tracking.html

[Build Bot]

Created attachment 226662 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

Created attachment 226685 [details]
Patch

[Build Bot]

Comment on attachment 226685 [details]
Patch

Attachment 226685 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5960730264207360

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html
fast/css/resize-corner-tracking-transformed.html
fast/css/resize-corner-tracking.html

[Build Bot]

Created attachment 226691 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 226685 [details]
Patch

Attachment 226685 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4760750761443328

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html
fast/css/resize-corner-tracking-transformed.html
fast/css/resize-corner-tracking.html

[Build Bot]

Created attachment 226696 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

Created attachment 226903 [details]
Patch

[zalan]

(In reply to comment #52)
> Created an attachment (id=226903) [details]
> Patch

Please make sure that the patch passes EWS.

(In reply to comment #53)
> (In reply to comment #52)
> > Created an attachment (id=226903) [details] [details]
> > Patch
> 
> Please make sure that the patch passes EWS.

(In reply to comment #53)
> (In reply to comment #52)
> > Created an attachment (id=226903) [details] [details]
> > Patch
> 
> Please make sure that the patch passes EWS.

Hi Zalan. Forgot to reset the review flag after uploading the patch.Will make sure that patch passrs EWS.

Created attachment 227019 [details]
Patch

[Build Bot]

Comment on attachment 227019 [details]
Patch

Attachment 227019 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5410420131102720

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html

[Build Bot]

Created attachment 227022 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227019 [details]
Patch

Attachment 227019 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5040623446917120

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html

[Build Bot]

Created attachment 227029 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227019 [details]
Patch

Attachment 227019 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5699184472621056

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html

[Build Bot]

Created attachment 227036 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 227019 [details]
Patch

Attachment 227019 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4547468859539456

New failing tests:
fast/css/resize-corner-tracking-transformed-iframe.html

[Build Bot]

Created attachment 227037 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

Created attachment 228508 [details]
Patch

(In reply to comment #64)
> Created an attachment (id=228508) [details]
> Patch

Hi Zalan and Simon can you please review. Sorry about the delay.Thanks

[zalan]

Comment on attachment 228508 [details]
Patch

Looks good!

[Simon Fraser (smfr)]

Comment on attachment 228508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228508&action=review

> Source/WebCore/page/EventHandler.cpp:1825
> -        m_resizeLayer->resize(mouseEvent, m_offsetFromResizeCorner);
> +        m_resizeLayer->resize(mouseEvent);

Now that you are no longer keeping m_offsetFromResizeCorner around. what is the behavior when the user clicks in the resize control but not at the very corner? This should *not* result in a size change until the mouse starts moving, but I suspect that it will with your patch; m_offsetFromResizeCorner I think tracked the delta between the mouse and the actual corner.

[Darin Adler]

Comment on attachment 228508 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228508&action=review

>> Source/WebCore/page/EventHandler.cpp:1825
>> +        m_resizeLayer->resize(mouseEvent);
> 
> Now that you are no longer keeping m_offsetFromResizeCorner around. what is the behavior when the user clicks in the resize control but not at the very corner? This should *not* result in a size change until the mouse starts moving, but I suspect that it will with your patch; m_offsetFromResizeCorner I think tracked the delta between the mouse and the actual corner.

I’d go further and say that it’s also an important feature that resizing be done so that the mouse stays in the same location relative to the resize corner, unless we hit minimums or maximums.

[zalan]

(In reply to comment #67)
> (From update of attachment 228508 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228508&action=review
> 
> > Source/WebCore/page/EventHandler.cpp:1825
> > -        m_resizeLayer->resize(mouseEvent, m_offsetFromResizeCorner);
> > +        m_resizeLayer->resize(mouseEvent);
> 
> Now that you are no longer keeping m_offsetFromResizeCorner around. what is the behavior when the user clicks in the resize control but not at the very corner? This should *not* result in a size change until the mouse starts moving, but I suspect that it will with your patch; m_offsetFromResizeCorner I think tracked the delta between the mouse and the actual corner.

The mouse click will not result in a size change, but when the user starts moving, the resize corner will immediately jump to the cursor position.(drag point). Currently we don't snap the resize corner to the cursor, but maintain the delta as the cursor moves around.

(In reply to comment #67)
> (From update of attachment 228508 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228508&action=review
> 
> > Source/WebCore/page/EventHandler.cpp:1825
> > -        m_resizeLayer->resize(mouseEvent, m_offsetFromResizeCorner);
> > +        m_resizeLayer->resize(mouseEvent);
> 
> Now that you are no longer keeping m_offsetFromResizeCorner around. what is the behavior when the user clicks in the resize control but not at the very corner? This should *not* result in a size change until the mouse starts moving, but I suspect that it will with your patch; m_offsetFromResizeCorner I think tracked the delta between the mouse and the actual corner.

Hi Simon the click does not result in size change. Till the mouse move does not happen resize also does not happen.

[zalan]

(In reply to comment #70)
> (In reply to comment #67)
> > (From update of attachment 228508 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=228508&action=review
> > 
> > > Source/WebCore/page/EventHandler.cpp:1825
> > > -        m_resizeLayer->resize(mouseEvent, m_offsetFromResizeCorner);
> > > +        m_resizeLayer->resize(mouseEvent);
> > 
> > Now that you are no longer keeping m_offsetFromResizeCorner around. what is the behavior when the user clicks in the resize control but not at the very corner? This should *not* result in a size change until the mouse starts moving, but I suspect that it will with your patch; m_offsetFromResizeCorner I think tracked the delta between the mouse and the actual corner.
> 
> Hi Simon the click does not result in size change. Till the mouse move does not happen resize also does not happen.
We probably want to preserve the current behavior so that the resize corner does not snap to the cursor when the user starts moving it.

[Darin Adler]

(In reply to comment #71)
> We probably want to preserve the current behavior so that the resize corner does not snap to the cursor when the user starts moving it.

That’s right. We do want to preserve that behavior.

[Gurpreet]

(In reply to comment #72)
> (In reply to comment #71)
> > We probably want to preserve the current behavior so that the resize corner does not snap to the cursor when the user starts moving it.
> 
> That’s right. We do want to preserve that behavior.

Hi Darin and Zalan what I understood is that the resize corner point should not jump or resize to the cursor position even if my cursor is not at the resize corner point, if that is the concern then I tested it not happening.

[Darin Adler]

(In reply to comment #73)
> (In reply to comment #72)
> > (In reply to comment #71)
> > > We probably want to preserve the current behavior so that the resize corner does not snap to the cursor when the user starts moving it.
> > 
> > That’s right. We do want to preserve that behavior.
> 
> Hi Darin and Zalan what I understood is that the resize corner point should not jump or resize to the cursor position even if my cursor is not at the resize corner point, if that is the concern then I tested it not happening.

If you start resizing with the cursor in the lower right hand corner of the resize box, then the cursor should remain in that corner during resizing.

If you start resizing with the cursor in the upper left hand corner of the resize box, then the cursor should remain in that corner during resizing.

That’s the concern, not “jumping or resizing to the cursor position”.

I can tell you that it can’t do that without remembering the offset of the cursor within the resize box when resizing began. You have removed the code that remembers that offset, so it doesn’t work right any more.

[Gurpreet]

(In reply to comment #74)
> (In reply to comment #73)
> > (In reply to comment #72)
> > > (In reply to comment #71)
> > > > We probably want to preserve the current behavior so that the resize corner does not snap to the cursor when the user starts moving it.
> > > 
> > > That’s right. We do want to preserve that behavior.
> > 
> > Hi Darin and Zalan what I understood is that the resize corner point should not jump or resize to the cursor position even if my cursor is not at the resize corner point, if that is the concern then I tested it not happening.
> 
> If you start resizing with the cursor in the lower right hand corner of the resize box, then the cursor should remain in that corner during resizing.
> 
> If you start resizing with the cursor in the upper left hand corner of the resize box, then the cursor should remain in that corner during resizing.
> 
> That’s the concern, not “jumping or resizing to the cursor position”.
> 
> I can tell you that it can’t do that without remembering the offset of the cursor within the resize box when resizing began. You have removed the code that remembers that offset, so it doesn’t work right any more.

Since we figured out that newOffset would do the resizing so the formula for calculating the difference i.e     LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(minimumSize) - currentSize;
was removed and so were the other unused variables like adjustedOldOffset which was calculated based on oldOffset and hence I made all the changes.

[Darin Adler]

(In reply to comment #75)
> Since we figured out that newOffset would do the resizing

That’s the problem. We can’t resize based only on newOffset, because that loses the information about what the offset was originally.

Lets say that the box we are resizing starts at 0,0, has an initial size of 100x100, and the resize corner is a 16x16 rectangle in the lower right corner. Imagine that when the mouse goes down, the cursor is in the center of the box. That’s at 92,92. Then it’s dragged to 42,42. The user moved the mouse by 50 pixels in each direction and the new size of the box needs to be 50x50 instead of the original 100x100.

Note that in this case, newOffset is going to be -58,-58. The code in your patch will end up giving the box a size of 42x42, but that is wrong, the size should be 50x50.

What math can possibly correctly give us the size 50x50 if we don’t save that original 92,92 position?

The old code would have saved an oldOffset of -8,-8, and then the code would take that into account and compute the correct size, 50x50.

[Ognian]

C'mon what happens here? Chrome has in the meantime fixed this,
Firefox was from the beginning ok, so Safari is the last one to fix...
I was hoping that this will get in iOS8, so that some layouts for sigle page apps get quite easy and performant...
Any chance for this fixed soon?
Thanks
Ognian

[Gurpreet]

Hi Darin, Simon and Zalan this issue is already fixed in Blink https://codereview.chromium.org/239983004. This does the min-width and min-height calculation everytime on resize which I had earlier submitted in the 6th patch. 
If the changes are fine shall I merge and close this issue?

[Simon Fraser (smfr)]

(In reply to comment #77)
> I was hoping that this will get in iOS8, so that some layouts for sigle page apps get quite easy and performant...

How does this affect iOS pages? They don't allow dynamic resizing of textareas etc. Can you point to a page that shows the issue?

[Simon Fraser (smfr)]

(In reply to comment #78)
> Hi Darin, Simon and Zalan this issue is already fixed in Blink https://codereview.chromium.org/239983004. This does the min-width and min-height calculation everytime on resize which I had earlier submitted in the 6th patch. 
> If the changes are fine shall I merge and close this issue?

In the last round of review we weren't convinced that you had the correct resizing behavior. Has this changed?

[Ognian]

(In reply to comment #79)
> (In reply to comment #77)
> > I was hoping that this will get in iOS8, so that some layouts for sigle page apps get quite easy and performant...
> 
> How does this affect iOS pages? They don't allow dynamic resizing of textareas etc. Can you point to a page that shows the issue?


My previous statement wasn't precise, yes iOS pages do not allow yet dynamic resize, but when setting a width or a height they do respect it.
If I use now the same layout on a desktop, then I can't reduce the height or width.
And before starting to adjust the things with my own js I can also stick to the frameworks I'm using until now...

[Gurpreet]

(In reply to comment #80)
> (In reply to comment #78)
> > Hi Darin, Simon and Zalan this issue is already fixed in Blink https://codereview.chromium.org/239983004. This does the min-width and min-height calculation everytime on resize which I had earlier submitted in the 6th patch. 
> > If the changes are fine shall I merge and close this issue?
> 
> In the last round of review we weren't convinced that you had the correct resizing behavior. Has this changed?

Hi Simon. No I have not uploaded any new patch since the last comment. I am not very clear since whatever I understood I have already submitted patch.
The Blink patch I can see everytime the min-width and min-height is being calculated. Did you see the Blink patch?

[Darin Adler]

Comment on attachment 228508 [details]
Patch

review- because of the resizing behavior change; we can’t just delete m_offsetFromResizeCorner because its purpose remains

[Gurpreet]

(In reply to comment #83)
> (From update of attachment 228508 [details])
> review- because of the resizing behavior change; we can’t just delete m_offsetFromResizeCorner because its purpose remains

Hi Darin. I agree with what you are saying.
But my concern was https://codereview.chromium.org/239983004. The bug got fixed and in that recalc of min-width and min-height is called again and again.

[zalan]

(In reply to comment #84)
> (In reply to comment #83)
> > (From update of attachment 228508 [details] [details])
> > review- because of the resizing behavior change; we can’t just delete m_offsetFromResizeCorner because its purpose remains
> 
> Hi Darin. I agree with what you are saying.
> But my concern was https://codereview.chromium.org/239983004. The bug got fixed and in that recalc of min-width and min-height is called again and again.
But didn't that changeset get reverted? The last entry on that bug is to revert the patch.

[Gurpreet]

> But didn't that changeset get reverted? The last entry on that bug is to revert the patch.
Oh sorry I missed that. I had mentioned the blink issue link sometime back.

[Florian Rivoal]

This bugs is still here, and in the meanwhile, the specification got clarifications, which in the context of this bug boil down to:
* Must allow resizing down from the initial size
* Must respect min-height min-width

Not respecting this causes two tests of the CSS-UI-3 test suite to fail:

https://test.csswg.org/harness/test/css-ui-3_dev/single/resize-019/format/html5/

https://test.csswg.org/harness/test/css-ui-3_dev/single/resize-020/format/html5/

This passes in Firefox, but is one of the last things in CSS-UI-3 not to have 2 implementations.

(link to the spec for reference: https://drafts.csswg.org/css-ui-3/#resize)

[Radar WebKit Bug Importer]

<rdar://problem/32981111>

[Jouni Koivuviita]

> This passes in Firefox, but is one of the last things in CSS-UI-3 not to have 2 implementations.

I’m not sure about the tests, but at least this has been fixed in Chrome, where you can resize an element below its initial size. Safari is still broken in that regard, that it does not respect min-width|height.

The specs is very clear on this now:

> The user agent must allow the user to resize the element with no other constraints than what is imposed by min-width, max-width, min-height, and max-height.
https://drafts.csswg.org/css-ui/#resize

[Simon Fraser (smfr)]

This was fixed via bug 135335.

*** This bug has been marked as a duplicate of bug 135335 ***