Improvements in queryCommandState and queryCommandValue

In the past two months, I spent some time improving WebKit's queryCommandState and queryCommandValue implementations. Here are a list of important queryCommand*-related bugs I fixed recently:

Every change except the last two will be shipped with Chromium version 9 (build 552), and the rest are included in Chromium dev-channels and on WebKit nightly, and will be shipped with Chromium version 10. Amongst the most important bugs I have fixed are 21680, 21033, and 45910 (+ 46954).

Background Color #

The bug 21680 is about WebKit not returning the correct value when querying BackColor. Namely, when an element has a transparent background color, return rgba(0, 0, 0, 0), which matches exactly what you would get through window.getComputedStyle. For example, if we had <span style="background-color:blue;">[hello]</span> where [ and ] denotes the start and the end of the current selection respectively, then document.queryCommandValue('BackColor') returns rgb(0, 0, 255) just as expected.

However, this implementation is quite cumbersome for editing purposes because the default value of CSS background-color property is transparent. Consider the following markup:

<span style="background-color: blue;">hello <em>[world]</em></span>

In this case, the computed style of the container node at the start of selection is em, and document.queryCommandValue('BackColor') returns rgb(0, 0, 0, 0) because em's background-color value is transparent. However, the effective background color of the text world that user sees is blue. In new implementation of document.queryCommandValue('BackColor'), we fixed the bug by walking the DOM tree upwards whenever current node's background-color has the alpha value of 0. Let's take a look at the code I wrote:

if (propertyID == CSSPropertyBackgroundColor && (m_frame->selection()->isRange() || hasTransparentBackgroundColor(selectionStyle.get()))) {
    RefPtr<Range> range(m_frame->selection()->toNormalizedRange());
    ExceptionCode ec = 0;
    for (Node* ancestor = range->commonAncestorContainer(ec); ancestor; ancestor = ancestor->parentNode()) {
        selectionStyle = computedStyle(ancestor)->copy();
        if (!hasTransparentBackgroundColor(selectionStyle.get())) {
            value = selectionStyle->getPropertyValue(CSSPropertyBackgroundColor);
            break;
        }
    }
}

We walk the DOM tree from the common ancestor of the selection's range upwards until we hit a node with non-transparent background color. That means, we return blue for the two following cases:

<span style="background-color:blue;">[hello <span style="background-color:red;">world</span>]</span>
<span style="background-color:blue;">[hello <span style="background-color:red;">wor]ld</span></span>

The new behavior is consistent with Firefox and Internet Explorer as far as I checked. However, there is one pitfall to this implementation. None of browsers we checked as well as WebKit does not take position: absolute into account. What that means is that if queryCommandValue('BackColor') is called on an absolutely positioned element, it will return the background color of some ancestor of the selection range's common ancestor, which is not necessarily behind the text when rendered.

Font Size #

What the bug 21033 complained was simple. document.queryCommandValue('FontSize') returns font size in pixel on WebKit whereas it returns the legacy font size on Firefox and Internet Explorer. This indeed made sense because our implementation defaults to the computed style, and computed style's font-size is always in pixel.

The conversion between legacy font size (integers between 1 and 7 what font element's size attributes takes) and pixel equivalent is non-trivial. It depends on whether or not we're in quirks mode and whether or not the font in use is fixed width. We even have a big table in CSSStyleSelector.cpp to convert between these two measures (sorry, CSSStyleSelector.cpp is too big for trac to open, so I'd give you Gecko's code).

In our new implementation of queryCommandValue('FontSize'), I implemented the inverse function using this conversion table and always returns integers between 1 and 7. Since it's the conversion table is shared with our rendering engine, you can trust what queryCommandValue('FontSize') returns now.

Justify Left, Right, Center, and Full #

In the bugs 45910 and 46954, I implemented queryCommandState for JustifyLeft, JustifyRight, JustifyCenter, and JustifyFull. The original bug report for 45910 stated that queryCommandState always return false but that is rather an understatement. It had never been implemented before. It's just that we never throw exceptions when things aren't implemented in queryCommandState. Regardless, this feature has been implemented in WebKit now.