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/
Created attachment 116328 [details] A simple test-case
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.
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!).
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.
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.
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.
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.
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.
(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.
(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.
(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.
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.
(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.
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.
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
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.
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
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
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
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
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
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
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
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
(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
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
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
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
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
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
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
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
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
Comment on attachment 228508 [details] Patch Looks good!
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.
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.
(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.
(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.
(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.
(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.
(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.
(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.
(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.
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
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 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?
(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?
(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...
(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?
Comment on attachment 228508 [details] Patch review- because of the resizing behavior change; we can’t just delete m_offsetFromResizeCorner because its purpose remains
(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.
(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.
> 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.
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)
<rdar://problem/32981111>
> 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
This was fixed via bug 135335. *** This bug has been marked as a duplicate of bug 135335 ***