Changeset 203985 in webkit


Ignore:
Timestamp:
Aug 1, 2016, 1:49:53 PM (8 years ago)
Author:
Antti Koivisto
Message:

REGRESSION(r198943): drop-down menu navigation on fiddlevideo.com doesn't appear on iOS, works on OS X
​https://bugs.webkit.org/show_bug.cgi?id=160406
Source/WebCore:

rdar://problem/26310261

Reviewed by Simon Fraser.

On iOS we generate synthetic mouse events from taps. Click event is generated on tap only if the move event
doesn't produce visible changes to the document. This is important to make certain types of drop down menus
work.

The information on mutations is passed via WKContentObservation side channel which is updated from varous parts
of the code. Newly visible elements are detected CheckForVisibilityChangeOnRecalcStyle during style resolution.
This got broken by the style refactoring because it assumes that renderer is mutated along with style computation.
However mutation is now a separate step performed by RenderTreeUpdater.

Fix by moving CheckForVisibilityChange to RenderTreeUpdater.

Test: fast/content-observation/click-event-suppression-on-content-change.html

  • style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::Parent::Parent):
(WebCore::RenderTreeUpdater::updateElementRenderer):
(WebCore::RenderTreeUpdater::tearDownRenderer):
(WebCore::elementImplicitVisibility):
(WebCore::CheckForVisibilityChange::CheckForVisibilityChange):
(WebCore::CheckForVisibilityChange::~CheckForVisibilityChange):

  • style/StyleTreeResolver.cpp:

(WebCore::Style::TreeResolver::createAnimatedElementUpdate):
(WebCore::Style::TreeResolver::pushParent):
(WebCore::Style::TreeResolver::resolveComposedTree):
(WebCore::Style::elementImplicitVisibility): Deleted.
(WebCore::Style::CheckForVisibilityChangeOnRecalcStyle::CheckForVisibilityChangeOnRecalcStyle): Deleted.
(WebCore::Style::CheckForVisibilityChangeOnRecalcStyle::~CheckForVisibilityChangeOnRecalcStyle): Deleted.

LayoutTests:

Reviewed by Simon Fraser.

This stuff has had zero test coverage. Adding a basic UIScript based test.

  • TestExpectations:
  • fast/content-observation/click-event-suppression-on-content-change-expected.txt: Added.
  • fast/content-observation/click-event-suppression-on-content-change.html: Added.
Location:
trunk
Files:
3 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r203982 r203985  
     12016-08-01  Antti Koivisto  <antti@apple.com>
     2
     3        REGRESSION(r198943): drop-down menu navigation on fiddlevideo.com doesn't appear on iOS, works on OS X
     4        https://bugs.webkit.org/show_bug.cgi?id=160406
     5
     6        Reviewed by Simon Fraser.
     7
     8        This stuff has had zero test coverage. Adding a basic UIScript based test.
     9
     10        * TestExpectations:
     11        * fast/content-observation/click-event-suppression-on-content-change-expected.txt: Added.
     12        * fast/content-observation/click-event-suppression-on-content-change.html: Added.
     13
    1142016-08-01  Eric Carlson  <eric.carlson@apple.com>
    215

  • trunk/LayoutTests/TestExpectations

    r203922 r203985  
    2323fast/events/touch/ios [ Skip ]
    2424fast/scrolling/ios [ Skip ]
     25fast/content-observation [ Skip ]
    2526media/mac [ Skip ]
    2627

  • trunk/Source/WebCore/ChangeLog

    r203984 r203985  
     12016-08-01  Antti Koivisto  <antti@apple.com>
     2
     3        REGRESSION(r198943): drop-down menu navigation on fiddlevideo.com doesn't appear on iOS, works on OS X
     4        https://bugs.webkit.org/show_bug.cgi?id=160406
     5        rdar://problem/26310261
     6
     7        Reviewed by Simon Fraser.
     8
     9        On iOS we generate synthetic mouse events from taps. Click event is generated on tap only if the move event
     10        doesn't produce visible changes to the document. This is important to make certain types of drop down menus
     11        work.
     12
     13        The information on mutations is passed via WKContentObservation side channel which is updated from varous parts
     14        of the code. Newly visible elements are detected CheckForVisibilityChangeOnRecalcStyle during style resolution.
     15        This got broken by the style refactoring because it assumes that renderer is mutated along with style computation.
     16        However mutation is now a separate step performed by RenderTreeUpdater.
     17
     18        Fix by moving CheckForVisibilityChange to RenderTreeUpdater.
     19
     20        Test: fast/content-observation/click-event-suppression-on-content-change.html
     21
     22        * style/RenderTreeUpdater.cpp:
     23        (WebCore::RenderTreeUpdater::Parent::Parent):
     24        (WebCore::RenderTreeUpdater::updateElementRenderer):
     25        (WebCore::RenderTreeUpdater::tearDownRenderer):
     26        (WebCore::elementImplicitVisibility):
     27        (WebCore::CheckForVisibilityChange::CheckForVisibilityChange):
     28        (WebCore::CheckForVisibilityChange::~CheckForVisibilityChange):
     29        * style/StyleTreeResolver.cpp:
     30        (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
     31        (WebCore::Style::TreeResolver::pushParent):
     32        (WebCore::Style::TreeResolver::resolveComposedTree):
     33        (WebCore::Style::elementImplicitVisibility): Deleted.
     34        (WebCore::Style::CheckForVisibilityChangeOnRecalcStyle::CheckForVisibilityChangeOnRecalcStyle): Deleted.
     35        (WebCore::Style::CheckForVisibilityChangeOnRecalcStyle::~CheckForVisibilityChangeOnRecalcStyle): Deleted.
     36
    1372016-08-01  Eric Carlson  <eric.carlson@apple.com>
    238

  • trunk/Source/WebCore/style/RenderTreeUpdater.cpp

    r202716 r203985  
    3535#include "HTMLSlotElement.h"
    3636#include "InspectorInstrumentation.h"
     37#include "NodeRenderStyle.h"
    3738#include "PseudoElement.h"
    3839#include "RenderFullScreen.h"
    … …  
    4142#include "StyleTreeResolver.h"
    4243
     44#if PLATFORM(IOS)
     45#include "WKContentObservation.h"
     46#endif
     47
    4348namespace WebCore {
     49
     50#if PLATFORM(IOS)
     51class CheckForVisibilityChange {
     52public:
     53    CheckForVisibilityChange(const Element&);
     54    ~CheckForVisibilityChange();
     55
     56private:
     57    const Element& m_element;
     58    EDisplay m_previousDisplay;
     59    EVisibility m_previousVisibility;
     60    EVisibility m_previousImplicitVisibility;
     61};
     62#endif // PLATFORM(IOS)
    4463
    4564RenderTreeUpdater::Parent::Parent(ContainerNode& root)
    … …  
    243262void RenderTreeUpdater::updateElementRenderer(Element& element, Style::ElementUpdate& update)
    244263{
     264#if PLATFORM(IOS)
     265    CheckForVisibilityChange checkForVisibilityChange(element);
     266#endif
     267
    245268    bool shouldTearDownRenderers = update.change == Style::Detach && (element.renderer() || element.isNamedFlowContentElement());
    246269    if (shouldTearDownRenderers)
    … …  
    567590}
    568591
    569 }
     592#if PLATFORM(IOS)
     593static EVisibility elementImplicitVisibility(const Element& element)
     594{
     595    auto* renderer = element.renderer();
     596    if (!renderer)
     597        return VISIBLE;
     598
     599    auto& style = renderer->style();
     600
     601    auto width = style.width();
     602    auto height = style.height();
     603    if ((width.isFixed() && width.value() <= 0) || (height.isFixed() && height.value() <= 0))
     604        return HIDDEN;
     605
     606    auto top = style.top();
     607    auto left = style.left();
     608    if (left.isFixed() && width.isFixed() && -left.value() >= width.value())
     609        return HIDDEN;
     610
     611    if (top.isFixed() && height.isFixed() && -top.value() >= height.value())
     612        return HIDDEN;
     613    return VISIBLE;
     614}
     615
     616CheckForVisibilityChange::CheckForVisibilityChange(const Element& element)
     617    : m_element(element)
     618    , m_previousDisplay(element.renderStyle() ? element.renderStyle()->display() : NONE)
     619    , m_previousVisibility(element.renderStyle() ? element.renderStyle()->visibility() : HIDDEN)
     620    , m_previousImplicitVisibility(WKObservingContentChanges() && WKObservedContentChange() != WKContentVisibilityChange ? elementImplicitVisibility(element) : VISIBLE)
     621{
     622}
     623
     624CheckForVisibilityChange::~CheckForVisibilityChange()
     625{
     626    if (!WKObservingContentChanges())
     627        return;
     628    if (m_element.isInUserAgentShadowTree())
     629        return;
     630    auto* style = m_element.renderStyle();
     631    if (!style)
     632        return;
     633    if ((m_previousDisplay == NONE && style->display() != NONE) || (m_previousVisibility == HIDDEN && style->visibility() != HIDDEN)
     634        || (m_previousImplicitVisibility == HIDDEN && elementImplicitVisibility(m_element) == VISIBLE))
     635        WKSetObservedContentChange(WKContentVisibilityChange);
     636}
     637#endif
     638
     639}

  • trunk/Source/WebCore/style/StyleTreeResolver.cpp

    r202439 r203985  
    4848#include "Text.h"
    4949
    50 #if PLATFORM(IOS)
    51 #include "WKContentObservation.h"
    52 #endif
    53 
    5450namespace WebCore {
    5551
    … …  
    266262}
    267263
    268 #if PLATFORM(IOS)
    269 static EVisibility elementImplicitVisibility(const Element* element)
    270 {
    271     RenderObject* renderer = element->renderer();
    272     if (!renderer)
    273         return VISIBLE;
    274 
    275     auto& style = renderer->style();
    276 
    277     Length width(style.width());
    278     Length height(style.height());
    279     if ((width.isFixed() && width.value() <= 0) || (height.isFixed() && height.value() <= 0))
    280         return HIDDEN;
    281 
    282     Length top(style.top());
    283     Length left(style.left());
    284     if (left.isFixed() && width.isFixed() && -left.value() >= width.value())
    285         return HIDDEN;
    286 
    287     if (top.isFixed() && height.isFixed() && -top.value() >= height.value())
    288         return HIDDEN;
    289     return VISIBLE;
    290 }
    291 
    292 class CheckForVisibilityChangeOnRecalcStyle {
    293 public:
    294     CheckForVisibilityChangeOnRecalcStyle(Element* element, const RenderStyle* currentStyle)
    295         : m_element(element)
    296         , m_previousDisplay(currentStyle ? currentStyle->display() : NONE)
    297         , m_previousVisibility(currentStyle ? currentStyle->visibility() : HIDDEN)
    298         , m_previousImplicitVisibility(WKObservingContentChanges() && WKObservedContentChange() != WKContentVisibilityChange ? elementImplicitVisibility(element) : VISIBLE)
    299     {
    300     }
    301     ~CheckForVisibilityChangeOnRecalcStyle()
    302     {
    303         if (!WKObservingContentChanges())
    304             return;
    305         if (m_element->isInUserAgentShadowTree())
    306             return;
    307         auto* style = m_element->renderStyle();
    308         if (!style)
    309             return;
    310         if ((m_previousDisplay == NONE && style->display() != NONE) || (m_previousVisibility == HIDDEN && style->visibility() != HIDDEN)
    311             || (m_previousImplicitVisibility == HIDDEN && elementImplicitVisibility(m_element.get()) == VISIBLE))
    312             WKSetObservedContentChange(WKContentVisibilityChange);
    313     }
    314 private:
    315     RefPtr<Element> m_element;
    316     EDisplay m_previousDisplay;
    317     EVisibility m_previousVisibility;
    318     EVisibility m_previousImplicitVisibility;
    319 };
    320 #endif // PLATFORM(IOS)
    321 
    322264void TreeResolver::pushParent(Element& element, const RenderStyle& style, Change change)
    323265{
    … …  
    449391        bool shouldResolve = shouldResolveElement(element, parent.change) || affectedByPreviousSibling;
    450392        if (shouldResolve) {
    451 #if PLATFORM(IOS)
    452             CheckForVisibilityChangeOnRecalcStyle checkForVisibilityChange(&element, element.renderStyle());
    453 #endif
    454393            element.resetComputedStyle();
    455394
Note: See TracChangeset for help on using the changeset viewer.