Changeset 47599 in webkit


Ignore:
Timestamp:
Aug 20, 2009 3:22:42 PM (15 years ago)
Author:
eric@webkit.org
Message:

2009-08-20 Xiaomei Ji <xji@chromium.org>

Reviewed by Eric Seidel.

Fix "Chromium RTL autocomplete popup is not layout correctly".
https://bugs.webkit.org/show_bug.cgi?id=27889

The complete fix of the issue consists 2 parts: the patch in webkit
and the patch in Chromium's own code.

This webkit patch only affects Chromium autofill. It

  1. introduces a new flag in WebCore::PopupContainerSettings to distinguish whether the width of the drop-down should be restricted or not. For autofill, the width of the drop-down is restricted to be the same as that of the input field (the new flag is set in Chromium's own code). But width is not restricted for <select> (same as before).
  2. introduce a new flag in WebCore::PopContainerSettings to indicate what heuristics to use when displaying text in drop-down menu. For autofill, use drop-down item's directionality to display drop-down items. Previously, drop-down item is displayed in the its first strong directional character's directionality. (drop-down item's directionality is set in Chromium's own code. It is set the same as the directionality of the element. For autofill, it is the same directionality as that of the input field.) For <select>, still use the text's first strong directional character's directionality to display the text.

Since the patch only affects the chromium client, not webcore part or
other clients. No automatic tests is possible.

  • manual-tests/autofill-popup-width-and-item-direction.html: Added.
  • platform/chromium/PopupMenuChromium.cpp: (WebCore::): (WebCore::PopupListBox::paintRow): If list box width is restricted and an item is longer to fit in a list box, truncate it and draw part of the text and append ellipses. (WebCore::PopupListBox::layout): Restrict width of list box if applicable.
  • platform/chromium/PopupMenuChromium.h: (WebCore::PopupItem::PopupItem): style change. (WebCore::PopupContainerSettings::): Add 2 new flags in PopupContainerSetting to distinguish whether to restrict width of list box and in what directionality to display the text in drop-down.
Location:
trunk/WebCore
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebCore/ChangeLog

    r47596 r47599  
     12009-08-20  Xiaomei Ji  <xji@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Fix "Chromium RTL autocomplete popup is not layout correctly".
     6        https://bugs.webkit.org/show_bug.cgi?id=27889
     7
     8        The complete fix of the issue consists 2 parts: the patch in webkit
     9        and the patch in Chromium's own code.
     10
     11        This webkit patch only affects Chromium autofill. It
     12        1. introduces a new flag in WebCore::PopupContainerSettings to
     13           distinguish whether the width of the drop-down should be restricted
     14           or not.
     15           For autofill, the width of the drop-down is restricted to
     16           be the same as that of the input field (the new flag is set in
     17           Chromium's own code). But width is not restricted for <select> (same as before).
     18        2. introduce a new flag in WebCore::PopContainerSettings to
     19           indicate what heuristics to use when displaying text in drop-down menu.
     20           For autofill, use drop-down item's directionality to display drop-down items.
     21           Previously, drop-down item is displayed in the its first strong
     22           directional character's directionality.
     23           (drop-down item's directionality is set in Chromium's own code.
     24           It is set the same as the directionality of the element.
     25           For autofill, it is the same directionality as that of the input field.)
     26           For <select>, still use the text's first strong directional character's
     27           directionality to display the text.
     28
     29
     30        Since the patch only affects the chromium client, not webcore part or
     31        other clients. No automatic tests is possible.
     32
     33        * manual-tests/autofill-popup-width-and-item-direction.html: Added.
     34        * platform/chromium/PopupMenuChromium.cpp:
     35        (WebCore::):
     36        (WebCore::PopupListBox::paintRow): If list box width is restricted and
     37        an item is longer to fit in a list box, truncate it and draw part of the text and append ellipses.
     38        (WebCore::PopupListBox::layout): Restrict width of list box if applicable.
     39        * platform/chromium/PopupMenuChromium.h:
     40        (WebCore::PopupItem::PopupItem): style change.
     41        (WebCore::PopupContainerSettings::): Add 2 new flags in PopupContainerSetting to
     42        distinguish whether to restrict width of list box and
     43        in what directionality to display the text in drop-down.
     44
    1452009-08-20  Brian Weinstein  <bweinstein@apple.com>
    246
  • trunk/WebCore/platform/chromium/PopupMenuChromium.cpp

    r47150 r47599  
    5151#include "RenderTheme.h"
    5252#include "ScrollbarTheme.h"
     53#include "StringTruncator.h"
    5354#include "SystemTime.h"
    5455
     
    7374// This is the delegate used if none is provided.
    7475static const PopupContainerSettings dropDownSettings = {
    75     true, // focusOnShow
    76     true, // setTextOnIndexChange
    77     true, // acceptOnAbandon
    78     false // loopSelectionNavigation
     76    true,   // focusOnShow
     77    true,   // setTextOnIndexChange
     78    true,   // acceptOnAbandon
     79    false,  // loopSelectionNavigation
     80    false,  // restrictWidthOfListBox
     81    // display item text in its first strong directional character's directionality.
     82    PopupContainerSettings::FirstStrongDirectionalCharacterDirection,
    7983};
    8084
     
    819823    }
    820824   
     825    if (!style.isVisible())
     826        return;
     827
    821828    gc->setFillColor(textColor);
    822829
    823     // Bunch of shit to deal with RTL text...
     830    Font itemFont = getRowFont(rowIndex);
     831    // FIXME: http://crbug.com/19872 We should get the padding of individual option
     832    // elements.  This probably implies changes to PopupMenuClient.
     833    int textX = max(0, m_popupClient->clientPaddingLeft() - m_popupClient->clientInsetLeft());
     834    int textY = rowRect.y() + itemFont.ascent() + (rowRect.height() - itemFont.height()) / 2;
     835    // Prepare text to be drawn.
    824836    String itemText = m_popupClient->itemText(rowIndex);
     837    if (m_settings.restrictWidthOfListBox)  // truncate string to fit in.
     838        itemText = StringTruncator::rightTruncate(itemText, rowRect.width() - textX, itemFont);
    825839    unsigned length = itemText.length();
    826840    const UChar* str = itemText.characters();
    827 
    828     TextRun textRun(str, length, false, 0, 0, itemText.defaultWritingDirection() == WTF::Unicode::RightToLeft);
    829 
    830     // FIXME: http://b/1210481 We should get the padding of individual option
    831     // elements.  This probably implies changes to PopupMenuClient.
    832 
    833     // Draw the item text
    834     if (style.isVisible()) {
    835         Font itemFont = getRowFont(rowIndex);
    836         int textX = max(0, m_popupClient->clientPaddingLeft() - m_popupClient->clientInsetLeft());
    837         int textY = rowRect.y() + itemFont.ascent() + (rowRect.height() - itemFont.height()) / 2;
    838         gc->drawBidiText(itemFont, textRun, IntPoint(textX, textY));
    839     }
     841    // Prepare the directionality to draw text.
     842    bool rtl = false;
     843    if (m_settings.itemTextDirectionalityHint == PopupContainerSettings::DOMElementDirection)
     844        rtl = style.textDirection() == RTL;
     845    else if (m_settings.itemTextDirectionalityHint ==
     846             PopupContainerSettings::FirstStrongDirectionalCharacterDirection)
     847        rtl = itemText.defaultWritingDirection() == WTF::Unicode::RightToLeft;
     848    TextRun textRun(str, length, false, 0, 0, rtl);
     849    // Draw the item text.
     850    gc->drawBidiText(itemFont, textRun, IntPoint(textX, textY));
    840851}
    841852
     
    10921103        }
    10931104        // FIXME: http://b/1210481 We should get the padding of individual option elements.
    1094         paddingWidth = max(paddingWidth, 
     1105        paddingWidth = max(paddingWidth,
    10951106            m_popupClient->clientPaddingLeft() + m_popupClient->clientPaddingRight());
    10961107    }
    10971108
     1109    // Calculate scroll bar width.
    10981110    int windowHeight = 0;
    10991111
     
    11261138        scrollbarWidth = ScrollbarTheme::nativeTheme()->scrollbarThickness();
    11271139
    1128     int windowWidth = baseWidth + scrollbarWidth + paddingWidth;
    1129     int contentWidth = baseWidth;
    1130 
    1131     if (windowWidth < m_baseWidth) {
     1140    int windowWidth;
     1141    int contentWidth;
     1142    if (m_settings.restrictWidthOfListBox) {
    11321143        windowWidth = m_baseWidth;
    11331144        contentWidth = m_baseWidth - scrollbarWidth - paddingWidth;
    11341145    } else {
    1135         m_baseWidth = baseWidth;
     1146        windowWidth = baseWidth + scrollbarWidth + paddingWidth;
     1147        contentWidth = baseWidth;
     1148
     1149        if (windowWidth < m_baseWidth) {
     1150            windowWidth = m_baseWidth;
     1151            contentWidth = m_baseWidth - scrollbarWidth - paddingWidth;
     1152        } else
     1153            m_baseWidth = baseWidth;
    11361154    }
    11371155
  • trunk/WebCore/platform/chromium/PopupMenuChromium.h

    r45685 r47599  
    5353
    5454        PopupItem(const String& label, Type type)
    55             : label(label), type(type), yOffset(0) { }
     55            : label(label)
     56            , type(type)
     57            , yOffset(0)
     58        {
     59        }
    5660        String label;
    5761        Type type;
     
    8488        bool acceptOnAbandon;
    8589
    86         // Whether the we should move the selection to the first/last item when
     90        // Whether we should move the selection to the first/last item when
    8791        // the user presses down/up arrow keys and the last/first item is
    8892        // selected.
    8993        bool loopSelectionNavigation;
     94
     95        // Whether we should restrict the width of the PopupListBox or not.
     96        // Autocomplete popups are restricted, combo-boxes (select tags) aren't.
     97        bool restrictWidthOfListBox;
     98
     99        // A hint on the display directionality of the item text in popup menu.
     100        //
     101        // We could either display the items in the drop-down using its DOM element's
     102        // directionality, or we could display the items in the drop-down using heuristics:
     103        // such as in its first strong directionality character's direction.
     104        // Please refer to the discussion (especially comment #7 and #10) in
     105        // https://bugs.webkit.org/show_bug.cgi?id=27889 for details.
     106        enum DirectionalityHint {
     107            // Use the DOM element's directionality to display the item text in popup menu.
     108            DOMElementDirection,
     109            // Use the item text's first strong-directional character's directionality
     110            // to display the item text in popup menu.
     111            FirstStrongDirectionalCharacterDirection,
     112        };
     113        DirectionalityHint itemTextDirectionalityHint;
    90114    };
    91115
Note: See TracChangeset for help on using the changeset viewer.