Skip to content

Commit

Permalink
AX: The Pause All Animations and Play All Animations context menu ite…
Browse files Browse the repository at this point in the history
…ms appear too high in the list

https://bugs.webkit.org/show_bug.cgi?id=259158
rdar://111593440

Reviewed by Andres Gonzalez.

Typically, context menu items are ordered such that the things most
relevant to the target are first, with "global" items coming after.
For example, if I activate the context menu on a link, I expect
"Copy Link" to be at the top of the list.

Due to the location where PauseAllAnimations and PlayAllAnimations
context menu items are added in ContextMenuController::populate, they
are often too high in the list, sometimes appearing above these more
relevant actions.

With this patch, these items are now grouped with the PauseAnimation and
PlayAnimation context menu items if they are present, and placed at the
bottom of the list otherwise, better matching user expectation.

* Source/WebCore/page/ContextMenuController.cpp:
(WebCore::ContextMenuController::populate):

Canonical link: https://commits.webkit.org/266037@main
  • Loading branch information
twilco committed Jul 13, 2023
1 parent 0aebf75 commit b2ea0a1
Showing 1 changed file with 23 additions and 9 deletions.
32 changes: 23 additions & 9 deletions Source/WebCore/page/ContextMenuController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,19 @@ void ContextMenuController::populate()
bool systemAllowsAnimationControls = autoplayAnimatedImagesFunction && !autoplayAnimatedImagesFunction();
return systemAllowsAnimationControls || frame->page()->settings().allowAnimationControlsOverride();
};

bool shouldAddPlayAllPauseAllAnimationsItem = canAddAnimationControls();
auto addPlayAllPauseAllAnimationsItem = [&] () {
if (!shouldAddPlayAllPauseAllAnimationsItem)
return;
// Only add this item once.
shouldAddPlayAllPauseAllAnimationsItem = false;

if (frame->page()->imageAnimationEnabled())
appendItem(PauseAllAnimations, m_contextMenu.get());
else
appendItem(PlayAllAnimations, m_contextMenu.get());
};
#endif

auto selectedText = m_context.hitTestResult().selectedText();
Expand Down Expand Up @@ -1096,6 +1109,8 @@ void ContextMenuController::populate()
appendItem(PauseAnimation, m_contextMenu.get());
else
appendItem(PlayAnimation, m_contextMenu.get());
// If the individual animation control action is available, group the Pause All Animations / Play All Animations action with it.
addPlayAllPauseAllAnimationsItem();
}
#endif // ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
}
Expand All @@ -1104,15 +1119,6 @@ void ContextMenuController::populate()
#endif
}

#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
if (canAddAnimationControls()) {
if (frame->page()->imageAnimationEnabled())
appendItem(PauseAllAnimations, m_contextMenu.get());
else
appendItem(PlayAllAnimations, m_contextMenu.get());
}
#endif

URL mediaURL = m_context.hitTestResult().absoluteMediaURL();
if (!mediaURL.isEmpty()) {
if (!linkURL.isEmpty() || !imageURL.isEmpty())
Expand Down Expand Up @@ -1381,6 +1387,14 @@ void ContextMenuController::populate()
appendItem(ShareMenuItem, m_contextMenu.get());
}
}

#if ENABLE(ACCESSIBILITY_ANIMATION_CONTROL)
if (shouldAddPlayAllPauseAllAnimationsItem) {
appendItem(*separatorItem(), m_contextMenu.get());
addPlayAllPauseAllAnimationsItem();
appendItem(*separatorItem(), m_contextMenu.get());
}
#endif
}

void ContextMenuController::addDebuggingItems()
Expand Down

0 comments on commit b2ea0a1

Please sign in to comment.