Changeset 123572 in webkit


Ignore:
Timestamp:
Jul 24, 2012 9:51:55 PM (12 years ago)
Author:
hbono@chromium.org
Message:

Avoid moving child objects multiple times when vertical scrollbar are shown at the left side.
​https://bugs.webkit.org/show_bug.cgi?id=91756

Reviewed by Tony Chang.

Source/WebCore:

My r123067 moves the top-left origin of an RTL element right when its vertical
scrollbar is shown at its left side. (That is, r123067 moves all child objects
in the RTL element right.) This change also increases RenderBox::clientLeft()
at the same time, i.e. it also moves child objects right. Furthermore, my r109512
moves positioned objects in an RTL element right at the same time. This makes
WebKit move objects in an RTL element up to three times by the scrollbar width.
(Moving an absolute object right increases the scrollWidth value and it causes
this bug.) This change removes unnecessary code that moves objects right in my
r109512 and RenderBox::clientLeft().

Test: scrollbars/rtl/div-absolute.html

fast/block/float/026.html
fast/block/float/028.html
fast/overflow/unreachable-overflow-rtl-bug.html

  • dom/Element.cpp:

(WebCore::Element::clientLeft): Increase clientLeft value by the width of a vertical scrollbar as written in the CSSOM specification.

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::addOverflowFromPositionedObjects): Removed unnecessary code.
(WebCore::RenderBlock::determineLogicalLeftPositionForChild): Removed unnecessary code.

  • rendering/RenderBox.h:

(WebCore::RenderBox::clientLeft): Removed unnecessary code.

LayoutTests:

This change adds a test that compares CSSOM properties of an RTL element which
includes positioned objects with the CSSOM properties of an LTR one. This change
also uses clientLeft properties in offsetX-offsetY.html to remove a hard-coded
value in the test and adds rebaselined results for Windows.

  • fast/events/offsetX-offsetY.html: Replaced a hard-coded value 'borderLeft' with clientLeft.
  • platform/chromium-linux/fast/block/float/026-expected.png:
  • platform/chromium-linux/fast/block/float/028-expected.png:
  • platform/chromium-win/fast/block/float/026-expected.png:
  • platform/chromium-win/fast/block/float/028-expected.png:
  • platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.png:
  • platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt:
  • scrollbars/rtl/div-absolute-expected.txt: Added.
  • scrollbars/rtl/div-absolute.html: Added.
Location:
trunk
Files:
2 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r123571 r123572  
     12012-07-24  Hironori Bono  <hbono@chromium.org>
     2
     3        Avoid moving child objects multiple times when vertical scrollbar are shown at the left side.
     4        https://bugs.webkit.org/show_bug.cgi?id=91756
     5
     6        Reviewed by Tony Chang.
     7
     8        This change adds a test that compares CSSOM properties of an RTL element which
     9        includes positioned objects with the CSSOM properties of an LTR one. This change
     10        also uses clientLeft properties in offsetX-offsetY.html to remove a hard-coded
     11        value in the test and adds rebaselined results for Windows.
     12
     13        * fast/events/offsetX-offsetY.html: Replaced a hard-coded value 'borderLeft' with clientLeft.
     14        * platform/chromium-linux/fast/block/float/026-expected.png:
     15        * platform/chromium-linux/fast/block/float/028-expected.png:
     16        * platform/chromium-win/fast/block/float/026-expected.png:
     17        * platform/chromium-win/fast/block/float/028-expected.png:
     18        * platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.png:
     19        * platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt:
     20        * scrollbars/rtl/div-absolute-expected.txt: Added.
     21        * scrollbars/rtl/div-absolute.html: Added.
     22
    1232012-07-24  Dan Bernstein  <mitz@apple.com>
    224

  • trunk/LayoutTests/fast/events/offsetX-offsetY.html

    r121735 r123572  
    7777      positions = sumPositions(content);
    7878      var inside = document.getElementById('inside-overflow');
    79       var borderLeft = 2;
    80       clientX = positions.offsetLeft + borderLeft + content.clientWidth - inside.clientWidth - window.scrollX + offsetX;
     79      var overflow = document.getElementById('overflow');
     80      clientX = positions.offsetLeft + overflow.clientLeft + content.clientWidth - inside.clientWidth - window.scrollX + offsetX;
    8181      dispatchEvent(clientX, clientY, 'inside-overflow', offsetX, offsetY);
    8282

  • trunk/LayoutTests/platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt

    r109591 r123572  
    1212layer at (8,28) size 106x106 clip at (11,31) size 85x85 scrollWidth 220 scrollHeight 270
    1313  RenderBlock (relative positioned) {DIV} at (0,20) size 106x106 [border: (3px solid #000000)]
    14 layer at (8,154) size 106x106 clip at (26,157) size 85x85 scrollX 150 scrollWidth 235 scrollHeight 270
     14layer at (8,154) size 106x106 clip at (26,157) size 85x85 scrollX 135 scrollWidth 220 scrollHeight 270
    1515  RenderBlock (relative positioned) {DIV} at (0,146) size 106x106 [border: (3px solid #000000)]

  • trunk/Source/WebCore/ChangeLog

    r123571 r123572  
     12012-07-24  Hironori Bono  <hbono@chromium.org>
     2
     3        Avoid moving child objects multiple times when vertical scrollbar are shown at the left side.
     4        https://bugs.webkit.org/show_bug.cgi?id=91756
     5
     6        Reviewed by Tony Chang.
     7
     8        My r123067 moves the top-left origin of an RTL element right when its vertical
     9        scrollbar is shown at its left side. (That is, r123067 moves all child objects
     10        in the RTL element right.) This change also increases RenderBox::clientLeft()
     11        at the same time, i.e. it also moves child objects right. Furthermore, my r109512
     12        moves positioned objects in an RTL element right at the same time. This makes
     13        WebKit move objects in an RTL element up to three times by the scrollbar width.
     14        (Moving an absolute object right increases the scrollWidth value and it causes
     15        this bug.) This change removes unnecessary code that moves objects right in my
     16        r109512 and RenderBox::clientLeft().
     17
     18        Test: scrollbars/rtl/div-absolute.html
     19              fast/block/float/026.html
     20              fast/block/float/028.html
     21              fast/overflow/unreachable-overflow-rtl-bug.html
     22
     23        * dom/Element.cpp:
     24        (WebCore::Element::clientLeft): Increase clientLeft value by the width of a vertical scrollbar as written in the CSSOM specification.
     25        * rendering/RenderBlock.cpp:
     26        (WebCore::RenderBlock::addOverflowFromPositionedObjects): Removed unnecessary code.
     27        (WebCore::RenderBlock::determineLogicalLeftPositionForChild): Removed unnecessary code.
     28        * rendering/RenderBox.h:
     29        (WebCore::RenderBox::clientLeft): Removed unnecessary code.
     30
    1312012-07-24  Dan Bernstein  <mitz@apple.com>
    232

  • trunk/Source/WebCore/dom/Element.cpp

    r123071 r123572  
    413413    document()->updateLayoutIgnorePendingStylesheets();
    414414
    415     if (RenderBox* renderer = renderBox())
    416         return adjustForAbsoluteZoom(roundToInt(renderer->clientLeft()), renderer);
     415    if (RenderBox* renderer = renderBox()) {
     416        LayoutUnit clientLeft = renderer->clientLeft();
     417        if (renderer->style() && renderer->style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
     418            clientLeft += renderer->verticalScrollbarWidth();
     419        return adjustForAbsoluteZoom(roundToInt(clientLeft), renderer);
     420    }
    417421    return 0;
    418422}

  • trunk/Source/WebCore/rendering/RenderBlock.cpp

    r123571 r123572  
    16701670       
    16711671        // Fixed positioned elements don't contribute to layout overflow, since they don't scroll with the content.
    1672         if (positionedObject->style()->position() != FixedPosition) {
    1673             int x = positionedObject->x();
    1674             if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
    1675                 x -= verticalScrollbarWidth();
    1676             addOverflowFromChild(positionedObject, IntSize(x, positionedObject->y()));
    1677         }
     1672        if (positionedObject->style()->position() != FixedPosition)
     1673            addOverflowFromChild(positionedObject, IntSize(positionedObject->x(), positionedObject->y()));
    16781674    }
    16791675}
    … …  
    22092205{
    22102206    LayoutUnit startPosition = borderStart() + paddingStart();
    2211     if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
    2212         startPosition -= verticalScrollbarWidth();
    22132207    LayoutUnit totalAvailableLogicalWidth = borderAndPaddingLogicalWidth() + availableLogicalWidth();
    22142208

  • trunk/Source/WebCore/rendering/RenderBox.h

    r123067 r123572  
    198198    // More IE extensions.  clientWidth and clientHeight represent the interior of an object
    199199    // excluding border and scrollbar.  clientLeft/Top are just the borderLeftWidth and borderTopWidth.
    200     LayoutUnit clientLeft() const { return borderLeft() + (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft() ? verticalScrollbarWidth() : 0); }
     200    LayoutUnit clientLeft() const { return borderLeft(); }
    201201    LayoutUnit clientTop() const { return borderTop(); }
    202202    LayoutUnit clientWidth() const;
Note: See TracChangeset for help on using the changeset viewer.