Tuesday, October 12, 2010

Chrome and JQuery UI min-height issue

The JQuery UI library attempts to determine whether the browser can support the min-height propery by injecting a <div> tag into the body and checking the offsetHeight property, which represents the total height of the element (including the padding & margin). If the offsetHeight is equal to 100, then the $.support.minHeight is also set to true.

jQuery UI (v.1.8.5)
// support
$(function() {
 var div = document.createElement( "div" ),
  body = document.body;

 $.extend( div.style, {
  minHeight: "100px",
  height: "auto",
  padding: 0,
  borderWidth: 0
 });

 $.support.minHeight = body.appendChild( div ).offsetHeight === 100;
 // set display to none to avoid a layout bug in IE
 // http://dev.jquery.com/ticket/4014
 body.removeChild( div ).style.display = "none";
});

The problem with this approach is that if you use Chrome and zoom-out (57%, 69%, 83%), the offsetHeight will be 98, 99, and 99 instead, therefore causing the $.support.minHeight to not equal to 100.

This issue can ultimately affect the use of the dialog box. If the browser cannot support the minimum height, it will resort to Math.max() operations to determine what the height of the dialog content should be. Since nonContentHeight can easily equal to 100px, you end up needing to set a content height of a minimum of 150px.

The problem of course is that your dialog box cannot grow/shrink according to the size of your content using the height: auto property.

this.element
   .css(options.height === 'auto' ? {
     minHeight: Math.max(options.minHeight - nonContentHeight, 0),
     height: $.support.minHeight ? 'auto' :
      Math.max(options.minHeight - nonContentHeight, 0)
    } : {
     minHeight: 0,
     height: Math.max(options.height - nonContentHeight, 0)    
   })
   .show();


I checked through the Chromium source code (http://dev.chromium.org/Home) and can confirm that HTML elements are scaled according to some type of zoom factor:

WebKit/WebCore/dom/Element.cpp
int Element::offsetHeight()
{
    document()->updateLayoutIgnorePendingStylesheets();
    if (RenderBoxModelObject* rend = renderBoxModelObject())
        return adjustForAbsoluteZoom(rend->offsetHeight(), rend);
    return 0;
}
.
.

inline int adjustForAbsoluteZoom(int value, const RenderStyle* style)
{
    double zoomFactor = style->effectiveZoom();
    if (zoomFactor == 1)
        return value;
    // Needed because computeLengthInt truncates (rather than rounds) when scaling up.
    if (zoomFactor > 1)
        value++;

    return roundForImpreciseConversion(value / zoomFactor);
}



template inline T roundForImpreciseConversion(double value)
{
    // Dimension calculations are imprecise, often resulting in values of e.g.
    // 44.99998.  We need to go ahead and round if we're really close to the
    // next integer value.
    value += (value < 0) ? -0.01 : +0.01;
    return ((value > max) || (value < min)) ? 0 : static_cast(value);
}

src/third_party/WebKit/WebCore/rendering/style/RenderStyle.h:
  static float initialZoom() { return 1.0f; }


m_style->setEffectiveZoom(m_parentStyle ? m_parentStyle->effectiveZoom() : RenderStyle::initialZoom());


  float m_effectiveZoom;

m_effectiveZoom(RenderStyle::initialZoom())



class RenderStyle: public RefCounted {
.
.
float effectiveZoom() const { return rareInheritedData->m_effectiveZoom; }

float zoom() const { return visual->m_zoom; }
void setZoom(float f) { SET_VAR(visual, m_zoom, f); setEffectiveZoom(effectiveZoom() * zoom()); }
}


The PageZoom class (found in src/chrome/common/page_zoom.h) seems to trigger different page zoom levels:

class PageZoom {
 public:
  // This enum is the parameter to various text/page zoom commands so we know
  // what the specific zoom command is.
  enum Function {
    ZOOM_OUT = -1,
    RESET    = 0,
    ZOOM_IN  = 1,
  };

 private:
  DISALLOW_IMPLICIT_CONSTRUCTORS(PageZoom);
};

#endif  // CHROME_COMMON_PAGE_ZOOM_H_

Inside browser.cc, the ZOOM commands are declared:
void Browser::Zoom(PageZoom::Function zoom_function) {

// Zoom
  command_updater_.UpdateCommandEnabled(IDC_ZOOM_MENU, true);
  command_updater_.UpdateCommandEnabled(IDC_ZOOM_PLUS, true);
  command_updater_.UpdateCommandEnabled(IDC_ZOOM_NORMAL, true);
  command_updater_.UpdateCommandEnabled(IDC_ZOOM_MINUS, true);

}

WebKit seems to adjust the zoom level here:
chromium/src/chrome/renderer/render_view.cc
void RenderView::OnZoom(PageZoom::Function function) {
  if (!webview())  // Not sure if this can happen, but no harm in being safe.
    return;

  webview()->hidePopups();

  double old_zoom_level = webview()->zoomLevel();
  double zoom_level;
  if (function == PageZoom::RESET) {
    zoom_level = 0;
  } else if (static_cast(old_zoom_level) == old_zoom_level) {
    // Previous zoom level is a whole number, so just increment/decrement.
    zoom_level = old_zoom_level + function;
  } else {
    // Either the user hit the zoom factor limit and thus the zoom level is now
    // not a whole number, or a plugin changed it to a custom value.  We want
    // to go to the next whole number so that the user can always get back to
    // 100% with the keyboard/menu.
    if ((old_zoom_level > 1 && function > 0) ||
        (old_zoom_level < 1 && function < 0)) {
      zoom_level = static_cast(old_zoom_level + function);
    } else {
      // We're going towards 100%, so first go to the next whole number.
      zoom_level = static_cast(old_zoom_level);
    }
  }

So far I've noticed that the height of the <div> scales properly, but the offsetHeight still reports 1-2px less than the desired minimum height. Oh, here's the first set of patches that implemented full page zoom on WebKit:

https://bug-14998-attachments.webkit.org/attachment.cgi?id=19878
https://bugs.webkit.org/show_bug.cgi?id=14998

Update: The problem possibly related to using integers for zooming/rendering?

https://bugs.webkit.org/show_bug.cgi?id=60318

No comments:

Post a Comment