Textarea resize-able only to larger; min-height and min-width properly set

Source/core/rendering/RenderLayerScrollableArea.cpp

Index: Source/core/rendering/RenderLayerScrollableArea.cpp
diff --git a/Source/core/rendering/RenderLayerScrollableArea.cpp b/Source/core/rendering/RenderLayerScrollableArea.cpp
index 3e96139bd8e833d2ee5d60f9403dd430b7dd0ae4..8eab87e78be66c37c15eb4c55cf32443d15fecc7 100644
--- a/Source/core/rendering/RenderLayerScrollableArea.cpp
+++ b/Source/core/rendering/RenderLayerScrollableArea.cpp
@@ -716,6 +716,18 @@ void RenderLayerScrollableArea::updateAfterStyleChange(const RenderStyle* oldSty
     EOverflow overflowX = m_box->style()->overflowX();
     EOverflow overflowY = m_box->style()->overflowY();
 
+    Node* node = m_box->node();
+    if (node && node->isElementNode()) {
+        Element* element = toElement(node);
+        LayoutUnit minimumWidth = valueForLength(m_box->style()->logicalMinWidth(), m_box->logicalWidth());
+        LayoutUnit minimumHeight = valueForLength(m_box->style()->logicalMinHeight(), m_box->logicalHeight());
+        if (minimumWidth == 0)
+            minimumWidth = LayoutUnit(15);
+        if (minimumHeight == 0)
+            minimumHeight = LayoutUnit(15);

[Julien - ping for review, 2014/04/21 17:46:46]

I don't think we should only check for 0. We should just do a min between
min-height/min-width and our internal minimal size.

Please also: NO MAGIC CONSTANT. Add a meaningful variable to hold them along
with a comment on why you chose these values as you have a great explanation
behind them.

[harpreet.sk, 2014/04/24 15:28:27]

Now added a check doing a max between the internal min-height/min-width and set
min-height/min-width. 

Added a variable along with comment for storing internal minimum size.

+        element->setMinimumSizeForResizing(LayoutSize(minimumWidth, minimumHeight));

[Julien - ping for review, 2014/04/21 17:46:46]

The fact that we do a round-trip through the DOM is just plain wrong (this was
pointed out in the original bug ->
https://bugs.webkit.org/show_bug.cgi?id=12798).

We don't have to fix it here but I think we should just store the minimum size
either in RenderLayerScrollableArea or directly ScrollableArea. The second
choice is more appealing to me if we can prove that other classes deriving from
ScrollableArea (RenderListBox comes to my mind) are impacted by this bug too
(which I would expect). Would you care to file a bug about that (extra points if
you want to fix it too)?

[harpreet.sk, 2014/04/22 13:56:36]

The classes deriving from ScrollableArea other than RenderLayerScrollableArea
are:

1. RenderListBox: The current bug will not impact this class. It is showing
behaviour similar to firefox. If we only set min-width/min-height and do not set
width/height then min-width/min-height of listbox will be max of intrinsic
calculated min-width/min-height and style min-width/min-height.  If width is
also been set in style then maximum between style width and style min-width will
be taken and set as the min-width of listbox. Same behaviour is shown by
firefox. So i don't think so we need to store the minimum size for listbox.

2. PinchViewport

3. ScrollView

4 ScrollbarGroup

I dont think that above classes will be impacted by the bug so i think it will
better to store the minimumSizeForResizing in RenderLayerScrollableArea other
than ScrollableArea. 

Currently i think only RenderLayerScrollableArea is using
minimumSizeForResizing() of Element.cpp and no other derive class of
ScrollableArea is using it. So it would be better to include it in
RenderLayerScrollableArea. 

Please correct me if i am wrong.

[Julien - ping for review, 2014/04/22 22:35:12]

Your analysis is fine by me.

+    }
+
     // To avoid doing a relayout in updateScrollbarsAfterLayout, we try to keep any automatic scrollbar that was already present.
     bool needsHorizontalScrollbar = (hasHorizontalScrollbar() && overflowDefinesAutomaticScrollbar(overflowX)) || overflowRequiresScrollbar(overflowX);
     bool needsVerticalScrollbar = (hasVerticalScrollbar() && overflowDefinesAutomaticScrollbar(overflowY)) || overflowRequiresScrollbar(overflowY);
@@ -1358,8 +1370,7 @@ void RenderLayerScrollableArea::resize(const PlatformEvent& evt, const LayoutSiz
     newOffset.setHeight(newOffset.height() / zoomFactor);
 
     LayoutSize currentSize = LayoutSize(m_box->width() / zoomFactor, m_box->height() / zoomFactor);
-    LayoutSize minimumSize = element->minimumSizeForResizing().shrunkTo(currentSize);

[Julien - ping for review, 2014/04/21 17:46:46]

Don't we need the shrunkTo call ias zoomFactor can be as big as you want (it's
determined by 'factor' AFAICT)?

[harpreet.sk, 2014/04/24 15:28:27]

We do not need shrunkTo() as over here m_box->width()/m_box->height() value is
equal to (zoomFactor * currentWidth)/(zoomFactor * currentHeigt) so after
reducing m_box->width() by factor of zoom we will never get value smaller than
minimum size.

-    element->setMinimumSizeForResizing(minimumSize);
+    LayoutSize minimumSize = element->minimumSizeForResizing();
 
     LayoutSize adjustedOldOffset = LayoutSize(oldOffset.width() / zoomFactor, oldOffset.height() / zoomFactor);
     if (m_box->style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft()) {