From: Anna Koskinen Date: Wed, 12 Nov 2014 14:13:01 +0000 (+0200) Subject: Embedded ThemeResource should react to theme change (#15194) X-Git-Tag: 7.4.0.beta3~20 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=0651b9aa3b8880110eeac9a220e267daa04c9b4f;p=vaadin-framework.git Embedded ThemeResource should react to theme change (#15194) Change-Id: Ied78a8c999e592a944c27138e179c37a31a0af54 --- diff --git a/WebContent/VAADIN/themes/tests-components/images/logo.png b/WebContent/VAADIN/themes/tests-components/images/logo.png new file mode 100644 index 0000000000..9990cd2f28 Binary files /dev/null and b/WebContent/VAADIN/themes/tests-components/images/logo.png differ diff --git a/client/src/com/vaadin/client/ui/embedded/EmbeddedConnector.java b/client/src/com/vaadin/client/ui/embedded/EmbeddedConnector.java index cd4c79ccc6..38995cf800 100644 --- a/client/src/com/vaadin/client/ui/embedded/EmbeddedConnector.java +++ b/client/src/com/vaadin/client/ui/embedded/EmbeddedConnector.java @@ -32,6 +32,7 @@ import com.vaadin.client.Paintable; import com.vaadin.client.UIDL; import com.vaadin.client.VConsole; import com.vaadin.client.VTooltip; +import com.vaadin.client.communication.StateChangeEvent; import com.vaadin.client.ui.AbstractComponentConnector; import com.vaadin.client.ui.ClickEventHandler; import com.vaadin.client.ui.VEmbedded; @@ -46,11 +47,44 @@ import com.vaadin.ui.Embedded; public class EmbeddedConnector extends AbstractComponentConnector implements Paintable { + private Element resourceElement; + private ObjectElement objectElement; + private String resourceUrl; + @Override protected void init() { super.init(); } + @Override + public void onStateChanged(StateChangeEvent stateChangeEvent) { + super.onStateChanged(stateChangeEvent); + // if theme has changed the resourceUrl may need to be updated + updateResourceIfNecessary(); + } + + private void updateResourceIfNecessary() { + if (resourceElement != null || objectElement != null) { + String src = getResourceUrl("src"); + if (src != null && !src.isEmpty()) { + if (!src.equals(resourceUrl)) { + setResourceUrl(src); + } + } else if (resourceUrl != null && !resourceUrl.isEmpty()) { + setResourceUrl(""); + } + } + } + + private void setResourceUrl(String src) { + resourceUrl = src; + if (resourceElement != null) { + resourceElement.setAttribute("src", src); + } else if (objectElement != null) { + objectElement.setData(src); + } + } + @Override public void updateFromUIDL(UIDL uidl, ApplicationConnection client) { if (!isRealUpdate(uidl)) { @@ -102,8 +136,9 @@ public class EmbeddedConnector extends AbstractComponentConnector implements style.setProperty("width", getState().width); style.setProperty("height", getState().height); - DOM.setElementProperty(el, "src", - getWidget().getSrc(uidl, client)); + resourceElement = el; + objectElement = null; + setResourceUrl(getResourceUrl("src")); if (uidl.hasAttribute(EmbeddedConstants.ALTERNATE_TEXT)) { el.setPropertyString( @@ -133,8 +168,9 @@ public class EmbeddedConnector extends AbstractComponentConnector implements getWidget().browserElement = DOM.getFirstChild(getWidget() .getElement()); } - DOM.setElementAttribute(getWidget().browserElement, "src", - getWidget().getSrc(uidl, client)); + resourceElement = getWidget().browserElement; + objectElement = null; + setResourceUrl(getResourceUrl("src")); clearBrowserElement = false; } else { VConsole.error("Unknown Embedded type '" + getWidget().type @@ -163,15 +199,19 @@ public class EmbeddedConnector extends AbstractComponentConnector implements getWidget().addStyleName(VEmbedded.CLASSNAME + "-svg"); String data; Map parameters = VEmbedded.getParameters(uidl); + ObjectElement obj = Document.get().createObjectElement(); + resourceElement = null; if (parameters.get("data") == null) { - data = getWidget().getSrc(uidl, client); + objectElement = obj; + data = getResourceUrl("src"); + setResourceUrl(data); } else { + objectElement = null; data = "data:image/svg+xml," + parameters.get("data"); + obj.setData(data); } getWidget().setHTML(""); - ObjectElement obj = Document.get().createObjectElement(); obj.setType(mime); - obj.setData(data); if (!isUndefinedWidth()) { obj.getStyle().setProperty("width", "100%"); } diff --git a/server/src/com/vaadin/ui/Embedded.java b/server/src/com/vaadin/ui/Embedded.java index 1086da8d09..d83eef9c4d 100644 --- a/server/src/com/vaadin/ui/Embedded.java +++ b/server/src/com/vaadin/ui/Embedded.java @@ -78,11 +78,6 @@ public class Embedded extends AbstractComponent implements LegacyComponent { */ private int type = TYPE_OBJECT; - /** - * Source of the embedded object. - */ - private Resource source = null; - /** * Generic object attributes. */ @@ -418,7 +413,7 @@ public class Embedded extends AbstractComponent implements LegacyComponent { * @return the Resource */ public Resource getSource() { - return source; + return getResource("src"); } /** @@ -445,8 +440,8 @@ public class Embedded extends AbstractComponent implements LegacyComponent { * the source to set. */ public void setSource(Resource source) { - if (source != null && !source.equals(this.source)) { - this.source = source; + if (source != null && !source.equals(getSource())) { + setResource("src", source); final String mt = source.getMIMEType(); if (mimeType == null) { diff --git a/uitest/src/com/vaadin/tests/components/embedded/EmbeddedThemeResource.java b/uitest/src/com/vaadin/tests/components/embedded/EmbeddedThemeResource.java new file mode 100644 index 0000000000..ecc746b858 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/embedded/EmbeddedThemeResource.java @@ -0,0 +1,69 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.embedded; + +import com.vaadin.server.ThemeResource; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Embedded; +import com.vaadin.ui.Image; +import com.vaadin.ui.themes.Reindeer; + +/** + * Tests that {@link Embedded} uses correct theme when the theme is set with + * {@link #setTheme(String)}, and also updates correctly if theme is changed + * later. {@link Image} is used as the baseline for correct behaviour. + * + * @author Vaadin Ltd + */ +public class EmbeddedThemeResource extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + setTheme("tests-components"); + + addButton("Toggle theme", new Button.ClickListener() { + + @Override + public void buttonClick(ClickEvent event) { + if (Reindeer.THEME_NAME.equals(getTheme())) { + setTheme("tests-components"); + } else { + setTheme(Reindeer.THEME_NAME); + } + } + }); + + // let's show a simple themeresource + ThemeResource logoResource = new ThemeResource("images/logo.png"); + Embedded embedded = new Embedded("embedded:", logoResource); + Image image = new Image("image:", logoResource); + + addComponents(embedded, image); + } + + @Override + protected String getTestDescription() { + return "Tests that Embedded updates correctly when using setTheme(String)"; + } + + @Override + protected Integer getTicketNumber() { + return 15194; + } +} diff --git a/uitest/src/com/vaadin/tests/components/embedded/EmbeddedThemeResourceTest.java b/uitest/src/com/vaadin/tests/components/embedded/EmbeddedThemeResourceTest.java new file mode 100644 index 0000000000..f3dca71cad --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/embedded/EmbeddedThemeResourceTest.java @@ -0,0 +1,109 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.embedded; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.remote.DesiredCapabilities; +import org.openqa.selenium.support.ui.ExpectedCondition; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.EmbeddedElement; +import com.vaadin.testbench.elements.ImageElement; +import com.vaadin.tests.tb3.MultiBrowserTest; +import com.vaadin.tests.tb3.SingleBrowserTest; +import com.vaadin.ui.Embedded; + +/** + * Tests that {@link Embedded} uses correct theme when the theme is set with + * {@link #setTheme(String)}, and also updates correctly if theme is changed + * later. {@link Image} is used as the baseline for correct behaviour. + * + * @author Vaadin Ltd + */ +public class EmbeddedThemeResourceTest extends SingleBrowserTest { + + @Override + public List getBrowsersToTest() { + // Seems like stylesheet onload is not fired on PhantomJS + // https://github.com/ariya/phantomjs/issues/12332 + return Arrays.asList(MultiBrowserTest.Browser.FIREFOX + .getDesiredCapabilities()); + } + + @Before + @Override + public void setup() throws Exception { + super.setup(); + openTestURL(); + waitForElementPresent(By.className("v-embedded")); + } + + @Test + public void testInitialTheme() { + EmbeddedElement embedded = $(EmbeddedElement.class).first(); + ImageElement image = $(ImageElement.class).first(); + final String initial = image.getAttribute("src"); + + assertFalse( + "ThemeResource image source uses default theme instead of set theme.", + initial.contains("/reindeer/")); + assertThat( + "Embedded and Image aren't using the same source for the image despite sharing the ThemeResource.", + embedded.findElement(By.tagName("img")).getAttribute("src"), + is(initial)); + } + + @Test + public void testUpdatedTheme() { + EmbeddedElement embedded = $(EmbeddedElement.class).first(); + final ImageElement image = $(ImageElement.class).first(); + final String initial = image.getAttribute("src"); + + // update theme + $(ButtonElement.class).first().click(); + + waitUntil(new ExpectedCondition() { + @Override + public Boolean apply(WebDriver input) { + return !initial.equals(image.getAttribute("src")); + } + + @Override + public String toString() { + // Timed out after 10 seconds waiting for ... + return "image source to be updated (was: " + initial + ")"; + } + }); + + assertTrue("ThemeResource image source didn't update correctly.", image + .getAttribute("src").contains("/reindeer/")); + assertThat( + "Embedded and Image aren't using the same source for the image despite sharing the ThemeResource.", + embedded.findElement(By.tagName("img")).getAttribute("src"), + is(image.getAttribute("src"))); + } +}