Skip to content

GitLab

T
tangled
Commit 00af5e78 authored by jbroman@chromium.org's avatar jbroman@chromium.org
Browse files
Options

Reland HTMLPlugInElement: Use custom focus logic only when there is a plugin. 

This is a reland of https://codereview.chromium.org/717193003/
which was reverted due to a SyzyASAN failure:
https://code.google.com/p/chromium/issues/detail?id=433357

I'm not quite sure how my patch caused the issue, but reverting it did fix it.
It seems that layout was being triggered during isKeyboardFocusable(), during
which the plugin element itself was destroyed.

This changes that path to only use an existing plugin to check for keyboard
focusability, similar to other code in HTMLPlugInElement.

TEST=fast/plugins/plugin/placeholder-focus.html
BUG=364716

Review URL: https://codereview.chromium.org/732783002

git-svn-id: svn://svn.chromium.org/blink/trunk@185468 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 8345cea6
Hide whitespace changes
Inline Side-by-side
Showing with 74 additions and 7 deletions
third_party/WebKit/LayoutTests/fast/plugins/plugin-placeholder-focus-expected.txt 0 → 100644
View file @ 00af5e78
Ensures that elements within a plugin placeholder can be keyboard focused.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS document.activeElement is input1
PASS shadowRoot1.activeElement is null
PASS document.activeElement is plugin1
PASS shadowRoot1.activeElement is non-null.
PASS document.activeElement is input1
PASS shadowRoot1.activeElement is null
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/plugins/plugin-placeholder-focus.html 0 → 100644
View file @ 00af5e78
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<div id="description"></div>
<div id="console"></div>
<input id="input1">
<object id="plugin1" type="application/x-fake-plugin"></object>
<script>
description('Ensures that elements within a plugin placeholder can be keyboard focused.');
// Close buttons are focusable, so we expect focus to move into and out of the placeholder.
var input1 = document.getElementById("input1");
var plugin1 = document.getElementById("plugin1");
internals.forcePluginPlaceholder(plugin1, { closeable: true });
var shadowRoot1 = internals.youngestShadowRoot(plugin1);
input1.focus();
shouldBe("document.activeElement", "input1");
shouldBeNull("shadowRoot1.activeElement");
eventSender.keyDown("\t");
shouldBe("document.activeElement", "plugin1");
shouldBeNonNull("shadowRoot1.activeElement");
eventSender.keyDown("\t", ["shiftKey"]);
shouldBe("document.activeElement", "input1");
shouldBeNull("shadowRoot1.activeElement");
document.activeElement.blur();
</script>
third_party/WebKit/Source/core/html/HTMLObjectElement.cpp
View file @ 00af5e78
...@@ -437,7 +437,9 @@ bool HTMLObjectElement::appendFormData(FormDataList& encoding, bool) ...@@ -437,7 +437,9 @@ bool HTMLObjectElement::appendFormData(FormDataList& encoding, bool)
if (name().isEmpty()) if (name().isEmpty())
return false; return false;
Widget* widget = pluginWidget(); // Widget is needed immediately to satisfy cases like
// LayoutTests/plugins/form-value.html.
Widget* widget = pluginWidgetForJSBindings();
if (!widget || !widget->isPluginView()) if (!widget || !widget->isPluginView())
return false; return false;
String value; String value;
......
third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp
View file @ 00af5e78
...@@ -129,7 +129,9 @@ void HTMLPlugInElement::setPersistedPluginWidget(Widget* widget) ...@@ -129,7 +129,9 @@ void HTMLPlugInElement::setPersistedPluginWidget(Widget* widget)
bool HTMLPlugInElement::canProcessDrag() const bool HTMLPlugInElement::canProcessDrag() const
{ {
return pluginWidget() && pluginWidget()->isPluginView() && toPluginView(pluginWidget())->canProcessDrag(); if (Widget* widget = existingPluginWidget())
return widget->isPluginView() && toPluginView(widget)->canProcessDrag();
return false;
} }
bool HTMLPlugInElement::willRespondToMouseClickEvents() bool HTMLPlugInElement::willRespondToMouseClickEvents()
...@@ -310,7 +312,7 @@ SharedPersistent<v8::Object>* HTMLPlugInElement::pluginWrapper() ...@@ -310,7 +312,7 @@ SharedPersistent<v8::Object>* HTMLPlugInElement::pluginWrapper()
if (m_persistedPluginWidget) if (m_persistedPluginWidget)
plugin = m_persistedPluginWidget.get(); plugin = m_persistedPluginWidget.get();
else else
plugin = pluginWidget(); plugin = pluginWidgetForJSBindings();
if (plugin) if (plugin)
m_pluginWrapper = frame->script().createPluginWrapper(plugin); m_pluginWrapper = frame->script().createPluginWrapper(plugin);
...@@ -318,7 +320,14 @@ SharedPersistent<v8::Object>* HTMLPlugInElement::pluginWrapper() ...@@ -318,7 +320,14 @@ SharedPersistent<v8::Object>* HTMLPlugInElement::pluginWrapper()
return m_pluginWrapper.get(); return m_pluginWrapper.get();
} }
Widget* HTMLPlugInElement::pluginWidget() const Widget* HTMLPlugInElement::existingPluginWidget() const
{
if (RenderPart* renderPart = existingRenderPart())
return renderPart->widget();
return nullptr;
}
Widget* HTMLPlugInElement::pluginWidgetForJSBindings()
{ {
if (RenderPart* renderPart = renderPartForJSBindings()) if (RenderPart* renderPart = renderPartForJSBindings())
return renderPart->widget(); return renderPart->widget();
...@@ -390,14 +399,21 @@ RenderPart* HTMLPlugInElement::renderPartForJSBindings() const ...@@ -390,14 +399,21 @@ RenderPart* HTMLPlugInElement::renderPartForJSBindings() const
bool HTMLPlugInElement::isKeyboardFocusable() const bool HTMLPlugInElement::isKeyboardFocusable() const
{ {
if (useFallbackContent() || usePlaceholderContent())
return HTMLElement::isKeyboardFocusable();
if (!document().isActive()) if (!document().isActive())
return false; return false;
return pluginWidget() && pluginWidget()->isPluginView() && toPluginView(pluginWidget())->supportsKeyboardFocus();
if (Widget* widget = existingPluginWidget())
return widget->isPluginView() && toPluginView(widget)->supportsKeyboardFocus();
return false;
} }
bool HTMLPlugInElement::hasCustomFocusLogic() const bool HTMLPlugInElement::hasCustomFocusLogic() const
{ {
return !hasAuthorShadowRoot(); return !useFallbackContent() && !usePlaceholderContent();
} }
bool HTMLPlugInElement::isPluginElement() const bool HTMLPlugInElement::isPluginElement() const
......
third_party/WebKit/Source/core/html/HTMLPlugInElement.h
View file @ 00af5e78
...@@ -52,8 +52,16 @@ public: ...@@ -52,8 +52,16 @@ public:
#endif #endif
void resetInstance(); void resetInstance();
// Returns the existing plugin widget, if there is one.
Widget* existingPluginWidget() const;
// Returns the plugin widget, forcing layout and post-layout tasks
// to happen synchronously (e.g. for JS bindings).
// See also renderPartForJSBindings().
Widget* pluginWidgetForJSBindings();
SharedPersistent<v8::Object>* pluginWrapper(); SharedPersistent<v8::Object>* pluginWrapper();
Widget* pluginWidget() const;
NPObject* getNPObject(); NPObject* getNPObject();
bool canProcessDrag() const; bool canProcessDrag() const;
const String& url() const { return m_url; } const String& url() const { return m_url; }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment