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

Source/core/rendering/RenderLayerScrollableArea.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
  * Other contributors:
  *   Robert O'Callahan <roc+@cs.cmu.edu>
  *   David Baron <dbaron@fas.harvard.edu>
  *   Christian Biesinger <cbiesinger@web.de>
  *   Randall Jesup <rjesup@wgate.com>
@@ skipping 56 matching lines @@
 #include "platform/graphics/GraphicsContextStateSaver.h"
 #include "platform/graphics/GraphicsLayer.h"
 #include "platform/scroll/ScrollAnimator.h"
 #include "platform/scroll/ScrollbarTheme.h"
 #include "public/platform/Platform.h"
 
 namespace WebCore {
 
 const int ResizerControlExpandRatioForTouch = 2;
 
+// Default value is set to 15 as the default
+// minimum size used by firefox is 15x15.
+static const LayoutUnit defaultMinimumValueForResizing(15);
+static const LayoutSize defaultMinimumSizeForResizing(defaultMinimumValueForResizing, defaultMinimumValueForResizing);

[Julien - ping for review, 2014/04/28 17:07:29]

What I meant by 2 variables is minWidth and minHeight, instead of bundling the 2
together.

We can keep the internal logic to deal with LayoutSize though.

[harpreet.sk, 2014/04/29 08:57:43]

Added the variables defaultMinimumWidthForResizing and
defaultMinimumHeightForResizing.  

+
 RenderLayerScrollableArea::RenderLayerScrollableArea(RenderBox* box)
     : m_box(box)
     , m_inResizeMode(false)
     , m_scrollsOverflow(false)
     , m_scrollDimensionsDirty(true)
     , m_inOverflowRelayout(false)
     , m_needsCompositedScrolling(false)
     , m_forceNeedsCompositedScrolling(DoNotForceCompositedScrolling)
+    , m_minimumSizeForResizing(defaultMinimumSizeForResizing)
     , m_scrollCorner(0)
     , m_resizer(0)
 {
     ScrollableArea::setConstrainsScrollingToContentEdge(false);
 
     Node* node = m_box->node();
     if (node && node->isElementNode()) {
         // We save and restore only the scrollOffset as the other scroll values are recalculated.
         Element* element = toElement(node);
         m_scrollOffset = element->savedLayerScrollOffset();
@@ skipping 613 matching lines @@
 static bool overflowRequiresScrollbar(EOverflow overflow)
 {
     return overflow == OSCROLL;
 }
 
 static bool overflowDefinesAutomaticScrollbar(EOverflow overflow)
 {
     return overflow == OAUTO || overflow == OOVERLAY;
 }
 
+void RenderLayerScrollableArea::updateMinimumSizeForResizing()
+{
+    LayoutUnit minimumWidth;
+    LayoutUnit minimumHeight;
+
+    if (m_box->containingBlock()) {
+        minimumWidth = valueForLength(m_box->style()->logicalMinWidth(), m_box->containingBlock()->logicalWidth());
+        minimumHeight = valueForLength(m_box->style()->logicalMinHeight(), m_box->containingBlock()->logicalHeight());

[Julien - ping for review, 2014/04/28 17:07:29]

This should be in physical coordinates as m_minimumSizeForResizing is. FWIW the
current logic is confused as it uses LayoutSize (fractional pixel unit) and not
physical pixels. I think we should just make this logic consistent and use
integer / IntSize, not LayoutUnit / LayoutSize.

To ensure the mark, we should also add an orthogonal flow test.

[harpreet.sk, 2014/04/29 08:57:43]

Now using int for minimumWidth and minimumHeight and the api will return
IntSize. 

Also the orthogonal flow test has been added.

+    }
+
+    minimumWidth = std::max(minimumWidth, defaultMinimumSizeForResizing.width());
+    minimumHeight = std::max(minimumHeight, defaultMinimumSizeForResizing.height());

[Julien - ping for review, 2014/04/28 17:07:30]

I think this code would be better if you initialized |minimumWidth| and
|minimumHeight| to these minimum and then do the std::max if you have a
containing block (when do we not have a containing block btw?)

[harpreet.sk, 2014/04/29 08:57:43]

Actually i used containing block check earlier because browser was getting
crashed just on launch maybe because for some element containing block does not
exist. Now i had remove this containing block check as now the api
minimumSizeForResizing will be called in the resize api where we want to use
minimum size for resizing and not in the updateStyleChange api. It works fine
for all cases.  

+    m_minimumSizeForResizing = LayoutSize(minimumWidth, minimumHeight);
+}
+
 void RenderLayerScrollableArea::updateAfterStyleChange(const RenderStyle* oldStyle)
 {
     // List box parts handle the scrollbars by themselves so we have nothing to do.
     if (m_box->style()->appearance() == ListboxPart)
         return;
 
     // RenderView shouldn't provide scrollbars on its own.
     if (m_box->isRenderView())
         return;
 
     if (!m_scrollDimensionsDirty)
         updateScrollableAreaSet(hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow());
 
     EOverflow overflowX = m_box->style()->overflowX();
     EOverflow overflowY = m_box->style()->overflowY();
 
+    updateMinimumSizeForResizing();
+
     // 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);
     setHasHorizontalScrollbar(needsHorizontalScrollbar);
     setHasVerticalScrollbar(needsVerticalScrollbar);
 
     // With overflow: scroll, scrollbars are always visible but may be disabled.
     // When switching to another value, we need to re-enable them (see bug 11985).
     if (needsHorizontalScrollbar && oldStyle && oldStyle->overflowX() == OSCROLL && overflowX != OSCROLL) {
         ASSERT(hasHorizontalScrollbar());
@@ skipping 602 matching lines @@
         ASSERT_NOT_REACHED();
     }
 
     float zoomFactor = m_box->style()->effectiveZoom();
 
     LayoutSize newOffset = offsetFromResizeCorner(document.view()->windowToContents(pos));
     newOffset.setWidth(newOffset.width() / zoomFactor);
     newOffset.setHeight(newOffset.height() / zoomFactor);
 
     LayoutSize currentSize = LayoutSize(m_box->width() / zoomFactor, m_box->height() / zoomFactor);
-    LayoutSize minimumSize = element->minimumSizeForResizing().shrunkTo(currentSize);
-    element->setMinimumSizeForResizing(minimumSize);
 
     LayoutSize adjustedOldOffset = LayoutSize(oldOffset.width() / zoomFactor, oldOffset.height() / zoomFactor);
     if (m_box->style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft()) {
         newOffset.setWidth(-newOffset.width());
         adjustedOldOffset.setWidth(-adjustedOldOffset.width());
     }
 
-    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(minimumSize) - currentSize;
+    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(m_minimumSizeForResizing) - currentSize;

[Julien - ping for review, 2014/04/28 17:07:30]

We should probably not cache the value as resize() is a rare operation compared
to style changes.

That's why I would just change updateMinimumSizeForResizing() to
minimumSizeForResizing() and make it return the IntSize.

[harpreet.sk, 2014/04/29 08:57:43]

The api is changed to minimumSizeForResizing() and it return IntSize.

 
     bool isBoxSizingBorder = m_box->style()->boxSizing() == BORDER_BOX;
 
     EResize resize = m_box->style()->resize();
     if (resize != RESIZE_VERTICAL && difference.width()) {
         if (element->isFormControlElement()) {
             // Make implicit margins from the theme explicit (see <http://bugs.webkit.org/show_bug.cgi?id=9547>).
             element->setInlineStyleProperty(CSSPropertyMarginLeft, m_box->marginLeft() / zoomFactor, CSSPrimitiveValue::CSS_PX);
             element->setInlineStyleProperty(CSSPropertyMarginRight, m_box->marginRight() / zoomFactor, CSSPrimitiveValue::CSS_PX);
         }
@@ skipping 147 matching lines @@
 void RenderLayerScrollableArea::setForceNeedsCompositedScrolling(ForceNeedsCompositedScrollingMode mode)
 {
     if (m_forceNeedsCompositedScrolling == mode)
         return;
 
     m_forceNeedsCompositedScrolling = mode;
     layer()->didUpdateNeedsCompositedScrolling();
 }
 
 } // Namespace WebCore