Skip to content

GitLab

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

Implement support for closing shadow DOM plugin placeholders. 

This is based on the current heuristic in
plugins::PluginPlaceholder::HidePlugin
(components/plugins/renderer/plugin_placeholder.cc).

Tests are added to ensure that the placeholder closing code works as
expected, and to verify that the newly-added WebPluginPlaceholder method
is respected.

A crude UI is currently used; showing the X graphics currently used is blocked
on deciding how to load image resources in this environment.

BUG=364716

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

git-svn-id: svn://svn.chromium.org/blink/trunk@184852 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 6a6fa8f0
Hide whitespace changes
Inline Side-by-side
Showing with 258 additions and 5 deletions
third_party/WebKit/LayoutTests/fast/plugins/plugin-placeholder-close-expected.txt 0 → 100644
View file @ 309cd39e
Checks that the plugin placeholder close button works as expected.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS closeButton1.style.display is 'none'
PASS closeButton2.style.display is not 'none'
PASS plugin2.style.display is 'none'
PASS closeButton3.style.display is not 'none'
PASS ancestor3.style.display is 'none'
PASS closeButton4.style.display is not 'none'
PASS document.getElementById('ancestor4a').style.display is not 'none'
PASS document.getElementById('ancestor4b').style.display is not 'none'
PASS document.getElementById('ancestor4c').style.display is not 'none'
PASS document.getElementById('ancestor4d').style.display is not 'none'
PASS document.getElementById('ancestor4e').style.display is not 'none'
PASS closeButton5.style.display is not 'none'
PASS document.getElementById('ancestor5').style.display is not 'none'
PASS closeButton6.style.display is not 'none'
PASS document.getElementById('ancestor6').style.display is 'none'
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/plugins/plugin-placeholder-close.html 0 → 100644
View file @ 309cd39e
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<div id="description"></div>
<div id="console"></div>
<object id="plugin1" type="application/x-fake-plugin"></object>
<object id="plugin2" type="application/x-fake-plugin"></object>
<div id="ancestor3" style="width: 400px; height: 300px">
<div>
<object id="plugin3" type="application/x-fake-plugin" width="400" height="300"></object>
</div>
</div>
<!--
None of these ancestors should be hidden.
ancestor4a is not explicitly sized.
ancestor4b is explicitly sized, but only in one dimension.
ancestor4c is explicitly sized, but not in the element's inline style.
ancestor4d is explicitly sized to match, but not in pixels.
ancestor4e is explicitly sized, but does not match the plugin's attributes.
-->
<style>
#ancestor4c { width: 400px; height: 300px; }
</style>
<div id="ancestor4a">
<div id="ancestor4b" style="width: 400px">
<div id="ancestor4c">
<div id="ancestor4d" style="font-size: 100px; width: 4em; height: 3em">
<div id="ancestor4e" style="width: 300px; height: 400px">
<object id="plugin4" type="application/x-fake-plugin" width="400" height="300"></object>
</div>
</div>
</div>
</div>
</div>
<!-- The plugin's size is 300x150, but it was not explicitly specified in the presentation attributes. -->
<div id="ancestor5" style="width: 300px; height: 150px">
<object id="plugin5" type="application/x-fake-plugin"></object>
</div>
<div id="ancestor6" style="width: 400px; height: 300px">
<object id="plugin6" type="application/x-fake-plugin" width=" 400 px " height="300px"></object>
</div>
<script>
description("Checks that the plugin placeholder close button works as expected.");
// Non-closeable plugins should have no displayed close button.
var plugin1 = document.getElementById("plugin1");
internals.forcePluginPlaceholder(plugin1, { closeable: false });
var closeButton1 = internals.youngestShadowRoot(plugin1).querySelector("#plugin-placeholder-close-button");
shouldBe("closeButton1.style.display", "'none'");
// After a plugin is closed, it should have "display: none".
var plugin2 = document.getElementById("plugin2");
internals.forcePluginPlaceholder(plugin2, { closeable: true });
var closeButton2 = internals.youngestShadowRoot(plugin2).querySelector("#plugin-placeholder-close-button");
shouldNotBe("closeButton2.style.display", "'none'");
closeButton2.click();
shouldBe("plugin2.style.display", "'none'");
// If the plugin has an ancestor sized to match with inline style, the ancestor should also be hidden.
var plugin3 = document.getElementById("plugin3");
internals.forcePluginPlaceholder(plugin3, { closeable: true });
var closeButton3 = internals.youngestShadowRoot(plugin3).querySelector("#plugin-placeholder-close-button");
shouldNotBe("closeButton3.style.display", "'none'");
closeButton3.click();
var ancestor3 = document.getElementById("ancestor3");
shouldBe("ancestor3.style.display", "'none'");
// If the plugin has ancestors that don't meet this heuristic, they should be left alone.
var plugin4 = document.getElementById("plugin4");
internals.forcePluginPlaceholder(plugin4, { closeable: true });
var closeButton4 = internals.youngestShadowRoot(plugin4).querySelector("#plugin-placeholder-close-button");
shouldNotBe("closeButton4.style.display", "'none'");
closeButton4.click();
shouldNotBe("document.getElementById('ancestor4a').style.display", "'none'");
shouldNotBe("document.getElementById('ancestor4b').style.display", "'none'");
shouldNotBe("document.getElementById('ancestor4c').style.display", "'none'");
shouldNotBe("document.getElementById('ancestor4d').style.display", "'none'");
shouldNotBe("document.getElementById('ancestor4e').style.display", "'none'");
// Plugins with no explicit (presentation attribute) size should not hide ancestors.
var plugin5 = document.getElementById("plugin5");
internals.forcePluginPlaceholder(plugin5, { closeable: true });
var closeButton5 = internals.youngestShadowRoot(plugin5).querySelector("#plugin-placeholder-close-button");
shouldNotBe("closeButton5.style.display", "'none'");
closeButton5.click();
shouldNotBe("document.getElementById('ancestor5').style.display", "'none'");
// This should work even if the presentation attribute has spacing and "px".
var plugin6 = document.getElementById("plugin6");
internals.forcePluginPlaceholder(plugin6, { closeable: true });
var closeButton6 = internals.youngestShadowRoot(plugin6).querySelector("#plugin-placeholder-close-button");
shouldNotBe("closeButton6.style.display", "'none'");
closeButton6.click();
shouldBe("document.getElementById('ancestor6').style.display", "'none'");
</script>
third_party/WebKit/LayoutTests/fast/plugins/plugin-placeholder-structured-expected.html
View file @ 309cd39e
...@@ -11,3 +11,14 @@ ...@@ -11,3 +11,14 @@
</div> </div>
</div> </div>
</div> </div>
<div class="plugin">
<div class="plugin-placeholder">
<div class="plugin-placeholder-content">
<div class="plugin-placeholder-message">
Sorry, but your plugin couldn't be loaded.
</div>
<button>Close</button>
</div>
</div>
</div>
third_party/WebKit/LayoutTests/fast/plugins/plugin-placeholder-structured.html
View file @ 309cd39e
<!DOCTYPE html> <!DOCTYPE html>
<!-- The message inserted below should be shown with some suitable style. --> <style>
<object type="application/x-fake-plugin"></object> applet, embed, object { display: block; }
</style>
<!-- The placeholders inserted below should be shown with some suitable style. -->
<object id="plugin1" type="application/x-fake-plugin"></object>
<object id="plugin2" type="application/x-fake-plugin"></object>
<script> <script>
var plugin = document.querySelector("object"); internals.forcePluginPlaceholder(document.getElementById('plugin1'), { message: "Sorry, but your plugin couldn't be loaded." });
internals.forcePluginPlaceholder(plugin, { message: "Sorry, but your plugin couldn't be loaded." }); internals.forcePluginPlaceholder(document.getElementById('plugin2'), { message: "Sorry, but your plugin couldn't be loaded.", closeable: true });
</script> </script>
third_party/WebKit/Source/core/html/shadow/PluginPlaceholderElement.cpp
View file @ 309cd39e
...@@ -30,4 +30,10 @@ void PluginPlaceholderElement::setMessage(const String& message) ...@@ -30,4 +30,10 @@ void PluginPlaceholderElement::setMessage(const String& message)
ASSERT_UNUSED(success, success); ASSERT_UNUSED(success, success);
} }
void PluginPlaceholderElement::setIsCloseable(bool closeable)
{
bool success = V8PluginPlaceholderElement::PrivateScript::closeableAttributeSetter(document().frame(), this, closeable);
ASSERT_UNUSED(success, success);
}
} // namespace blink } // namespace blink
third_party/WebKit/Source/core/html/shadow/PluginPlaceholderElement.h
View file @ 309cd39e
...@@ -16,6 +16,7 @@ public: ...@@ -16,6 +16,7 @@ public:
static PassRefPtrWillBeRawPtr<PluginPlaceholderElement> create(Document&); static PassRefPtrWillBeRawPtr<PluginPlaceholderElement> create(Document&);
void setMessage(const String&); void setMessage(const String&);
void setIsCloseable(bool);
private: private:
explicit PluginPlaceholderElement(Document&); explicit PluginPlaceholderElement(Document&);
......
third_party/WebKit/Source/core/html/shadow/PluginPlaceholderElement.idl
View file @ 309cd39e
...@@ -6,5 +6,7 @@ ...@@ -6,5 +6,7 @@
NoInterfaceObject NoInterfaceObject
] interface PluginPlaceholderElement : HTMLDivElement { ] interface PluginPlaceholderElement : HTMLDivElement {
[ImplementedInPrivateScript, OnlyExposedToPrivateScript] attribute DOMString message; [ImplementedInPrivateScript, OnlyExposedToPrivateScript] attribute DOMString message;
[ImplementedInPrivateScript, OnlyExposedToPrivateScript] attribute boolean closeable;
[ImplementedInPrivateScript, OnlyExposedToPrivateScript] void createdCallback(); [ImplementedInPrivateScript, OnlyExposedToPrivateScript] void createdCallback();
}; };
third_party/WebKit/Source/core/html/shadow/PluginPlaceholderElement.js
View file @ 309cd39e
...@@ -33,15 +33,70 @@ installClass('PluginPlaceholderElement', function(PluginPlaceholderElementProtot ...@@ -33,15 +33,70 @@ installClass('PluginPlaceholderElement', function(PluginPlaceholderElementProtot
var messageElement = document.createElement('div'); var messageElement = document.createElement('div');
messageElement.id = 'plugin-placeholder-message'; messageElement.id = 'plugin-placeholder-message';
// FIXME: UI polish, l10n, etc. for the close button.
var closeButton = document.createElement('button');
closeButton.id = 'plugin-placeholder-close-button';
closeButton.textContent = 'Close';
closeButton.style.display = 'none';
closeButton.addEventListener('click', function() {
// FIXME: Record UMA Plugin_Hide_Click.
this.hide();
}.bind(this));
contentElement.appendChild(messageElement); contentElement.appendChild(messageElement);
contentElement.appendChild(closeButton);
this.appendChild(styleElement); this.appendChild(styleElement);
this.appendChild(contentElement); this.appendChild(contentElement);
this.messageElement = messageElement; this.messageElement = messageElement;
this.closeButton = closeButton;
};
PluginPlaceholderElementPrototype.hide = function() {
var host = (this.parentNode instanceof ShadowRoot) ? this.parentNode.host : this;
host.style.display = 'none';
// If we have a width and height, search for a parent (often <div>) with the
// same dimensions. If we find such a parent, hide that as well.
// This makes much more uncovered page content usable (including clickable)
// as opposed to merely visible.
// TODO(cevans) -- it's a foul heuristic but we're going to tolerate it for
// now for these reasons:
// 1) Makes the user experience better.
// 2) Foulness is encapsulated within this single function.
// 3) Confidence in no false positives.
// 4) Seems to have a good / low false negative rate at this time.
//
// This heuristic was copied from plugins::PluginPlaceholder::HidePlugin
// (src/components/plugins/renderer/plugin_placeholder.cc) and should be
// kept in sync with it until it goes away.
if (host.hasAttribute('width') && host.hasAttribute('height')) {
var presentationAttributeInPixels = function(attr) {
var match = host.getAttribute(attr).match(/^\s*(\d+)\s*(px)?\s*$/);
if (match)
return match[1] + 'px';
};
var width = presentationAttributeInPixels('width');
var height = presentationAttributeInPixels('height');
if (!width || !height)
return;
var element = host;
while (element instanceof Element) {
if (element.style.width == width && element.style.height == height)
element.style.display = 'none';
element = element.parentNode;
}
}
}; };
Object.defineProperty(PluginPlaceholderElementPrototype, 'message', { Object.defineProperty(PluginPlaceholderElementPrototype, 'message', {
get: function() { return this.messageElement.textContent; }, get: function() { return this.messageElement.textContent; },
set: function(message) { this.messageElement.textContent = message; }, set: function(message) { this.messageElement.textContent = message; },
}); });
Object.defineProperty(PluginPlaceholderElementPrototype, 'closeable', {
get: function() { return this.closeButton.style.display != 'none'; },
set: function(closeable) { this.closeButton.style.display = closeable ? '' : 'none'; },
});
}); });
third_party/WebKit/Source/core/plugins/testing/DictionaryPluginPlaceholder.h
View file @ 309cd39e
...@@ -24,6 +24,10 @@ public: ...@@ -24,6 +24,10 @@ public:
if (DictionaryHelper::get(options, "message", stringValue)) if (DictionaryHelper::get(options, "message", stringValue))
placeholder->setMessage(stringValue); placeholder->setMessage(stringValue);
bool booleanValue;
if (DictionaryHelper::get(options, "closeable", booleanValue))
placeholder->setIsCloseable(booleanValue);
return adoptPtrWillBeNoop(new DictionaryPluginPlaceholder(placeholder.release())); return adoptPtrWillBeNoop(new DictionaryPluginPlaceholder(placeholder.release()));
} }
......
third_party/WebKit/Source/web/PluginPlaceholderImpl.cpp
View file @ 309cd39e
...@@ -39,6 +39,7 @@ void PluginPlaceholderImpl::loadIntoContainer(ContainerNode& container) ...@@ -39,6 +39,7 @@ void PluginPlaceholderImpl::loadIntoContainer(ContainerNode& container)
void PluginPlaceholderImpl::update() void PluginPlaceholderImpl::update()
{ {
m_placeholderElement->setMessage(m_webPluginPlaceholder->message()); m_placeholderElement->setMessage(m_webPluginPlaceholder->message());
m_placeholderElement->setIsCloseable(m_webPluginPlaceholder->isCloseable());
} }
} // namespace blink } // namespace blink
third_party/WebKit/Source/web/PluginPlaceholderImplTest.cpp
View file @ 309cd39e
...@@ -5,7 +5,12 @@ ...@@ -5,7 +5,12 @@
#include "config.h" #include "config.h"
#include "web/PluginPlaceholderImpl.h" #include "web/PluginPlaceholderImpl.h"
#include "core/CSSPropertyNames.h"
#include "core/CSSValueKeywords.h"
#include "core/HTMLNames.h" #include "core/HTMLNames.h"
#include "core/css/CSSPrimitiveValue.h"
#include "core/css/CSSValue.h"
#include "core/css/StylePropertySet.h"
#include "core/dom/DocumentFragment.h" #include "core/dom/DocumentFragment.h"
#include "core/dom/TagCollection.h" #include "core/dom/TagCollection.h"
#include "core/testing/DummyPageHolder.h" #include "core/testing/DummyPageHolder.h"
...@@ -29,9 +34,13 @@ public: ...@@ -29,9 +34,13 @@ public:
virtual ~MockWebPluginPlaceholder() { } virtual ~MockWebPluginPlaceholder() { }
MOCK_CONST_METHOD0(message, WebString()); MOCK_CONST_METHOD0(message, WebString());
MOCK_CONST_METHOD0(isCloseable, bool());
private: private:
MockWebPluginPlaceholder() { } MockWebPluginPlaceholder()
{
ON_CALL(*this, message()).WillByDefault(Return(WebString()));
}
}; };
// Fixture which creates a dummy context for running these this test. // Fixture which creates a dummy context for running these this test.
...@@ -86,5 +95,34 @@ TEST_F(PluginPlaceholderImplTest, MessageDoesNotAcceptElements) ...@@ -86,5 +95,34 @@ TEST_F(PluginPlaceholderImplTest, MessageDoesNotAcceptElements)
EXPECT_FALSE(documentFragment().getElementById("sentinel")); EXPECT_FALSE(documentFragment().getElementById("sentinel"));
} }
bool isHiddenWithInlineStyle(Element* element)
{
if (!element->inlineStyle())
return false;
RefPtrWillBeRawPtr<CSSValue> value = element->inlineStyle()->getPropertyCSSValue(CSSPropertyDisplay);
return value && value->isPrimitiveValue() && toCSSPrimitiveValue(value.get())->getValueID() == CSSValueNone;
}
TEST_F(PluginPlaceholderImplTest, Closeable)
{
// The closing functionality of PluginPlaceholderElement is tested in
// LayoutTests/fast/plugins. This test only needs to ensure that the
// boolean in WebPluginPlaceholder is respected.
EXPECT_CALL(webPluginPlaceholder(), isCloseable()).WillOnce(Return(true));
pluginPlaceholder().loadIntoContainer(documentFragment());
RefPtrWillBeRawPtr<Element> closeButton = documentFragment().getElementById("plugin-placeholder-close-button");
ASSERT_NE(nullptr, closeButton);
EXPECT_FALSE(isHiddenWithInlineStyle(closeButton.get()));
}
TEST_F(PluginPlaceholderImplTest, NotCloseable)
{
EXPECT_CALL(webPluginPlaceholder(), isCloseable()).WillOnce(Return(false));
pluginPlaceholder().loadIntoContainer(documentFragment());
RefPtrWillBeRawPtr<Element> closeButton = documentFragment().getElementById("plugin-placeholder-close-button");
EXPECT_NE(nullptr, closeButton);
EXPECT_TRUE(isHiddenWithInlineStyle(closeButton.get()));
}
} // namespace } // namespace
} // namespace blink } // namespace blink
third_party/WebKit/public/web/WebPluginPlaceholder.h
View file @ 309cd39e
...@@ -16,6 +16,9 @@ public: ...@@ -16,6 +16,9 @@ public:
// Returns a plaintext message to be displayed. // Returns a plaintext message to be displayed.
virtual WebString message() const { return WebString(); } virtual WebString message() const { return WebString(); }
// If set, the placeholder will present UI to permit dismissing it.
virtual bool isCloseable() const { return false; }
}; };
} // namespace blink } // namespace blink
......
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