diff options
author | Sauli Tähkäpää <sauli@vaadin.com> | 2014-06-12 13:55:12 +0300 |
---|---|---|
committer | Sauli Tähkäpää <sauli@vaadin.com> | 2014-06-12 13:57:01 +0300 |
commit | 0c00164f368b4a09f766518f7d16618cb5ee25ab (patch) | |
tree | b8ae5c3ae5ef4fcf617e847e872b934124177c38 | |
parent | 0d0f2d10a7f0f3aa5cdc55420512feb5ad8429e6 (diff) | |
download | vaadin-framework-0c00164f368b4a09f766518f7d16618cb5ee25ab.tar.gz vaadin-framework-0c00164f368b4a09f766518f7d16618cb5ee25ab.zip |
Revert "Fix for 'Aborting layout after 100 passess' (#13359)"
Causes regression with IE8:
http://r2d2.devnet.vaadin.com:8111/viewLog.html?buildTypeId=Vaadin72_Vaadin72DevelopmentBuildTb2Tests&buildId=86020
Change-Id: I6d848777b28a1d3f27a25fec778cba8d68a45690
6 files changed, 23 insertions, 512 deletions
diff --git a/client/src/com/vaadin/client/LayoutManager.java b/client/src/com/vaadin/client/LayoutManager.java index e17c0233b3..bf79009f4c 100644 --- a/client/src/com/vaadin/client/LayoutManager.java +++ b/client/src/com/vaadin/client/LayoutManager.java @@ -36,7 +36,6 @@ import com.vaadin.client.ui.VNotification; import com.vaadin.client.ui.layout.ElementResizeEvent; import com.vaadin.client.ui.layout.ElementResizeListener; import com.vaadin.client.ui.layout.LayoutDependencyTree; -import com.vaadin.client.ui.orderedlayout.AbstractOrderedLayoutConnector; public class LayoutManager { private static final String LOOP_ABORT_MESSAGE = "Aborting layout after 100 passes. This would probably be an infinite loop."; @@ -721,16 +720,6 @@ public class LayoutManager { Profiler.enter("LayoutManager.measureConnector"); Element element = connector.getWidget().getElement(); MeasuredSize measuredSize = getMeasuredSize(connector); - if (isBrowserOptimizedMeasuringNeeded() - && isWrappedInsideExpandBlock(connector)) { - // this fixes zoom/sub-pixel issues with ie9+ and Chrome - // (#13359) - // Update measuring logic to round up and/or down depending on - // browser. - measuredSize.setMeasurer(new MeasuredSize.RoundingMeasurer()); - } else { - measuredSize.setMeasurer(null);// resets to default measurer - } MeasureResult measureResult = measuredAndUpdate(element, measuredSize); if (measureResult.isChanged()) { @@ -740,23 +729,6 @@ public class LayoutManager { Profiler.leave("LayoutManager.measureConnector"); } - /** - * Return true if browser may need some optimized width and height measuring - * for MeasuredSize. - * <p> - * Usually optimization is needed to avoid unnecessary scroll bars appearing - * in layouts caused by sub-pixel rounding. And to avoid LayoutManager - * doLayout() going into a loop trying to fit content with fractional size - * (percentages) inside a parent element and stopping only after safety - * iteration limit exceeds, also caused by sub-pixel rounding. - * <p> - * For internal use only. May be removed or replaced in the future. - */ - private static boolean isBrowserOptimizedMeasuringNeeded() { - return BrowserInfo.get().isChrome() - || (BrowserInfo.get().isIE() && !BrowserInfo.get().isIE8()); - } - private void onConnectorChange(ComponentConnector connector, boolean widthChanged, boolean heightChanged) { Profiler.enter("LayoutManager.onConnectorChange"); @@ -838,26 +810,6 @@ public class LayoutManager { return connector instanceof ManagedLayout; } - /** - * Is the given connector wrapped inside a 'expanding' content. Detected by - * checking if the connector's parent is AbstractOrderedLayoutConnector that - * expands. 'Expand' means that some layout slots may expand its content - * width or height to some percentage fraction. - * - * @param connector - * The connector to check - * @return True if connector is wrapped inside a - * AbstractOrderedLayoutConnector that expands - */ - private static boolean isWrappedInsideExpandBlock( - ComponentConnector connector) { - ServerConnector parent = connector.getParent(); - if (parent instanceof AbstractOrderedLayoutConnector) { - return ((AbstractOrderedLayoutConnector) parent).needsExpand(); - } - return false; - } - public void forceLayout() { ConnectorMap connectorMap = connection.getConnectorMap(); JsArrayObject<ComponentConnector> componentConnectors = connectorMap diff --git a/client/src/com/vaadin/client/MeasuredSize.java b/client/src/com/vaadin/client/MeasuredSize.java index 4005a2651d..2531ff9389 100644 --- a/client/src/com/vaadin/client/MeasuredSize.java +++ b/client/src/com/vaadin/client/MeasuredSize.java @@ -43,49 +43,6 @@ public class MeasuredSize { } } - public interface Measurer { - int measureHeight(Element element); - - int measureWidth(Element element); - } - - /** - * Default measurer rounds always up. - */ - public static class DefaultMeasurer implements Measurer { - - @Override - public int measureHeight(Element element) { - int requiredHeight = Util.getRequiredHeight(element); - return requiredHeight; - } - - @Override - public int measureWidth(Element element) { - int requiredWidth = Util.getRequiredWidth(element); - return requiredWidth; - } - } - - /** - * RoundingMeasurer measurer rounds up and down, optimized for different - * browsers. - */ - public static class RoundingMeasurer implements Measurer { - - @Override - public int measureHeight(Element element) { - double requiredHeight = Util.getPreciseRequiredHeight(element); - return Util.roundPreciseSize(requiredHeight); - } - - @Override - public int measureWidth(Element element) { - double requiredWidth = Util.getPreciseRequiredWidth(element); - return Util.roundPreciseSize(requiredWidth); - } - } - private int width = -1; private int height = -1; @@ -95,15 +52,6 @@ public class MeasuredSize { private FastStringSet dependents = FastStringSet.create(); - private Measurer measurer = new DefaultMeasurer(); - - public void setMeasurer(Measurer measurer) { - if (measurer == null) { - measurer = new DefaultMeasurer(); - } - this.measurer = measurer; - } - public int getOuterHeight() { return height; } @@ -288,7 +236,7 @@ public class MeasuredSize { Profiler.leave("Measure borders"); Profiler.enter("Measure height"); - int requiredHeight = measurer.measureHeight(element); + int requiredHeight = Util.getRequiredHeight(element); int marginHeight = sumHeights(margins); int oldHeight = height; int oldWidth = width; @@ -299,7 +247,7 @@ public class MeasuredSize { Profiler.leave("Measure height"); Profiler.enter("Measure width"); - int requiredWidth = measurer.measureWidth(element); + int requiredWidth = Util.getRequiredWidth(element); int marginWidth = sumWidths(margins); if (setOuterWidth(requiredWidth + marginWidth)) { debugSizeChange(element, "Width (outer)", oldWidth, width); diff --git a/client/src/com/vaadin/client/Util.java b/client/src/com/vaadin/client/Util.java index bec88032bb..e031b37422 100644 --- a/client/src/com/vaadin/client/Util.java +++ b/client/src/com/vaadin/client/Util.java @@ -627,48 +627,16 @@ public class Util { return reqHeight; } - /** - * Gets the border-box width for the given element, i.e. element width + - * border + padding. - * - * @param element - * The element to check - * @return The border-box width for the element - */ - public static double getPreciseRequiredWidth( - com.google.gwt.dom.client.Element element) { - double reqWidth; - if (browserUsesPreciseSizeByComputedStyle()) { - reqWidth = getPreciseWidthByComputedStyle(element); - } else { - reqWidth = getPreciseBoundingClientRectWidth(element); - } - return reqWidth; - } - - /** - * Gets the border-box height for the given element, i.e. element height + - * border + padding. - * - * @param element - * The element to check - * @return The border-box height for the element - */ - public static double getPreciseRequiredHeight( - com.google.gwt.dom.client.Element element) { - double reqHeight; - if (browserUsesPreciseSizeByComputedStyle()) { - reqHeight = getPreciseHeightByComputedStyle(element); + public static native int getRequiredWidthBoundingClientRect( + com.google.gwt.dom.client.Element element) + /*-{ + if (element.getBoundingClientRect) { + var rect = element.getBoundingClientRect(); + return Math.ceil(rect.right - rect.left); } else { - reqHeight = getPreciseBoundingClientRectHeight(element); + return element.offsetWidth; } - return reqHeight; - } - - public static int getRequiredWidthBoundingClientRect( - com.google.gwt.dom.client.Element element) { - return (int) Math.ceil(getPreciseBoundingClientRectWidth(element)); - } + }-*/; public static native int getRequiredHeightComputedStyle( com.google.gwt.dom.client.Element element) @@ -710,10 +678,18 @@ public class Util { return Math.ceil(width+border+padding); }-*/; - public static int getRequiredHeightBoundingClientRect( - com.google.gwt.dom.client.Element element) { - return (int) Math.ceil(getPreciseBoundingClientRectHeight(element)); - } + public static native int getRequiredHeightBoundingClientRect( + com.google.gwt.dom.client.Element element) + /*-{ + var height; + if (element.getBoundingClientRect != null) { + var rect = element.getBoundingClientRect(); + height = Math.ceil(rect.bottom - rect.top); + } else { + height = element.offsetHeight; + } + return height; + }-*/; public static int getRequiredWidth(Widget widget) { return getRequiredWidth(widget.getElement()); @@ -724,128 +700,6 @@ public class Util { } /** - * Rounds pixel size up or down depending on the browser. - * <p> - * <li> - * IE9, IE10 - rounds always down to closest integer - * <li> - * Others - rounds half up to closest integer (1.49 -> 1.0, 1.5 -> 2.0) - * - * @param preciseSize - * Decimal number to round. - * @return Rounded integer - */ - public static int roundPreciseSize(double preciseSize) { - boolean roundAlwaysDown = BrowserInfo.get().isIE9() - || BrowserInfo.get().isIE10(); - if (roundAlwaysDown) { - return (int) preciseSize; - } else { - // round to closest integer - return (int) Math.round(preciseSize); - } - } - - /** - * Returns getBoundingClientRect.width, if supported. Otherwise return - * offsetWidth. Includes the padding, scrollbar, and the border. Excludes - * the margin. - * - * @param elem - * Target element to measure - */ - private static native double getPreciseBoundingClientRectWidth(Element elem) - /*-{ - try { - if (elem.getBoundingClientRect) { - return elem.getBoundingClientRect().width; - } else { - return (elem.offsetWidth) || 0; - } - } catch (e) { - // JS exception is thrown if the elem is not attached to the document. - return 0; - } - }-*/; - - /** - * Returns getBoundingClientRect.height, if supported. Otherwise return - * offsetHeight. Includes the padding, scrollbar, and the border. Excludes - * the margin. - * - * @param elem - * Target element to measure - */ - private static native double getPreciseBoundingClientRectHeight(Element elem) - /*-{ - try { - if (elem.getBoundingClientRect) { - return elem.getBoundingClientRect().height; - } else { - return (elem.offsetHeight) || 0; - } - } catch (e) { - // JS exception is thrown if the elem is not attached to the document. - return 0; - } - }-*/; - - /** - * Parse computed styles to get precise width for the given element. - * Excludes the margin. Fallback to offsetWidth if computed styles is not - * defined, or if width can't be read or it's not fractional. - * - * @param elem - * Target element to measure - */ - private static native double getPreciseWidthByComputedStyle( - com.google.gwt.dom.client.Element elem) - /*-{ - var cs = elem.ownerDocument.defaultView.getComputedStyle(elem); - var size = cs && cs.getPropertyValue('width') || ''; - if (size && size.indexOf('.') > -1) { - size = parseFloat(size) - + parseInt(cs.getPropertyValue('padding-left')) - + parseInt(cs.getPropertyValue('padding-right')) - + parseInt(cs.getPropertyValue('border-left-width')) - + parseInt(cs.getPropertyValue('border-right-width')); - } else { - size = elem.offsetWidth; - } - return size; - }-*/; - - /** - * Parse computed styles to get precise height for the given element. - * Excludes the margin. Fallback to offsetHeight if computed styles is not - * defined, or if height can't be read or it's not fractional. - * - * @param elem - * Target element to measure - */ - private static native double getPreciseHeightByComputedStyle( - com.google.gwt.dom.client.Element elem) - /*-{ - var cs = elem.ownerDocument.defaultView.getComputedStyle(elem); - var size = cs && cs.getPropertyValue('height') || ''; - if (size && size.indexOf('.') > -1) { - size = parseFloat(size) - + parseInt(cs.getPropertyValue('padding-top')) - + parseInt(cs.getPropertyValue('padding-bottom')) - + parseInt(cs.getPropertyValue('border-top-width')) - + parseInt(cs.getPropertyValue('border-bottom-width')); - } else { - size = elem.offsetHeight; - } - return size; - }-*/; - - /** For internal use only. May be removed or replaced in the future. */ - private static boolean browserUsesPreciseSizeByComputedStyle() { - return BrowserInfo.get().isIE9(); - } - - /** * Detects what is currently the overflow style attribute in given element. * * @param pe diff --git a/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java b/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java index c20eda71dc..0c09ae49c6 100644 --- a/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java +++ b/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java @@ -551,10 +551,8 @@ public abstract class AbstractOrderedLayoutConnector extends /** * Does the layout need to expand? - * <p> - * For internal use only. May be removed or replaced in the future. */ - public boolean needsExpand() { + private boolean needsExpand() { return needsExpand; } diff --git a/uitest/src/com/vaadin/tests/components/orderedlayout/OrderedLayoutExpand.java b/uitest/src/com/vaadin/tests/components/orderedlayout/OrderedLayoutExpand.java deleted file mode 100644 index e51e9a49a5..0000000000 --- a/uitest/src/com/vaadin/tests/components/orderedlayout/OrderedLayoutExpand.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * 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.orderedlayout; - -import com.vaadin.server.VaadinRequest; -import com.vaadin.tests.components.AbstractTestUI; -import com.vaadin.ui.Alignment; -import com.vaadin.ui.Component; -import com.vaadin.ui.HorizontalLayout; -import com.vaadin.ui.Label; -import com.vaadin.ui.UI; - -@SuppressWarnings("serial") -public class OrderedLayoutExpand extends AbstractTestUI { - - @Override - protected void setup(VaadinRequest request) { - - ThreeColumnLayout layout = new ThreeColumnLayout( - createLabelWithUndefinedSize("first component"), - createLabelWithUndefinedSize("second component"), - createLabelWithUndefinedSize("third component")); - - addComponent(layout); - setWidth("600px"); - - if (UI.getCurrent().getPage().getWebBrowser().isChrome()) { - getPage().getStyles().add("body { zoom: 1.10; }"); - } - } - - @Override - protected String getTestDescription() { - StringBuilder sb = new StringBuilder( - "You should not see 'Aborting layout after 100 passes.' error with debug mode and "); - if (UI.getCurrent().getPage().getWebBrowser().isChrome()) { - sb.append(" Zoom is "); - sb.append("1.10"); - } else { - sb.append(" zoom level 95%."); - } - return sb.toString(); - } - - @Override - protected Integer getTicketNumber() { - return 13359; - } - - private Label createLabelWithUndefinedSize(String caption) { - Label label = new Label(caption); - label.setSizeUndefined(); - return label; - } - - private static class ThreeColumnLayout extends HorizontalLayout { - public ThreeColumnLayout(Component leftComponent, - Component centerComponent, Component rightComponent) { - setWidth("100%"); - setMargin(true); - Component left = createLeftHolder(leftComponent); - this.addComponent(left); - setComponentAlignment(left, Alignment.MIDDLE_LEFT); - setExpandRatio(left, 1f); - Component center = createCenterHolder(centerComponent); - this.addComponent(center); - setComponentAlignment(center, Alignment.MIDDLE_CENTER); - setExpandRatio(center, 0f); - Component right = createRightHolder(rightComponent); - this.addComponent(right); - setComponentAlignment(right, Alignment.MIDDLE_RIGHT); - setExpandRatio(right, 1f); - } - - private Component createLeftHolder(Component c) { - HorizontalLayout hl = new HorizontalLayout(); - hl.addComponent(c); - hl.setWidth(100, Unit.PERCENTAGE); - return hl; - } - - private Component createCenterHolder(Component c) { - HorizontalLayout hl = new HorizontalLayout(); - hl.addComponent(c); - hl.setComponentAlignment(c, Alignment.MIDDLE_CENTER); - return hl; - } - - private Component createRightHolder(Component c) { - HorizontalLayout hl = new HorizontalLayout(); - hl.addComponent(c); - hl.setWidth(100, Unit.PERCENTAGE); - return hl; - } - } -} diff --git a/uitest/src/com/vaadin/tests/components/orderedlayout/OrderedLayoutExpandTest.java b/uitest/src/com/vaadin/tests/components/orderedlayout/OrderedLayoutExpandTest.java deleted file mode 100644 index 46b5f6b8b0..0000000000 --- a/uitest/src/com/vaadin/tests/components/orderedlayout/OrderedLayoutExpandTest.java +++ /dev/null @@ -1,132 +0,0 @@ -/* - * 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.orderedlayout; - -import java.util.List; - -import org.junit.Assert; -import org.junit.Test; -import org.openqa.selenium.By; -import org.openqa.selenium.Keys; -import org.openqa.selenium.NoSuchElementException; -import org.openqa.selenium.WebElement; -import org.openqa.selenium.remote.DesiredCapabilities; - -import com.vaadin.tests.tb3.MultiBrowserTest; - -public class OrderedLayoutExpandTest extends MultiBrowserTest { - - /* - * (non-Javadoc) - * - * @see com.vaadin.tests.tb3.MultiBrowserTest#getBrowsersToTest() - */ - @Override - public List<DesiredCapabilities> getBrowsersToTest() { - List<DesiredCapabilities> browsersToTest = super.getBrowsersToTest(); - - // Not sure why, but IE11 might give the following error message: - // "org.openqa.selenium.WebDriverException: Unexpected error launching - // Internet Explorer. Browser zoom level was set to 75%. It should be - // set to 100%". - - // setting "ignoreZoomSetting" capability to true seems to help, but if - // it keeps bugging, just skip the IE11 completely bu uncommenting the - // line below. - // browsersToTest.remove(Browser.IE11.getDesiredCapabilities()); - Browser.IE11.getDesiredCapabilities().setCapability( - "ignoreZoomSetting", true); - - // Can't test with IE8 with a zoom level other than 100%. IE8 could be - // removed to speed up the test run. - // browsersToTest.remove(Browser.IE8.getDesiredCapabilities()); - - return browsersToTest; - } - - @Test - public void testNoAbortingLayoutAfter100PassesError() throws Exception { - setDebug(true); - openTestURL(); - - if (!getDesiredCapabilities().equals( - Browser.CHROME.getDesiredCapabilities())) { - // Chrome uses css to to set zoom level to 110% that is known to - // cause issues with the test app. - // Other browsers tries to set browser's zoom level directly. - WebElement html = driver.findElement(By.tagName("html")); - // reset to 100% just in case - html.sendKeys(Keys.chord(Keys.CONTROL, "0")); - // zoom browser to 75% (ie) or 90% (FF). It depends on browser - // how much "Ctrl + '-'" zooms out. - html.sendKeys(Keys.chord(Keys.CONTROL, Keys.SUBTRACT)); - } - - // open debug window's Examine component hierarchy tab - openDebugExamineComponentHierarchyTab(); - - // click "Check layouts for potential problems" button - clickDebugCheckLayoutsForPotentialProblems(); - - // find div containing a successful layout analyze result - WebElement pass = findLayoutAnalyzePassElement(); - // find div containing a error message with - // "Aborting layout after 100 passess" message. - WebElement error = findLayoutAnalyzeAbortedElement(); - - Assert.assertNull(error); - Assert.assertNotNull(pass); - - if (!getDesiredCapabilities().equals( - Browser.CHROME.getDesiredCapabilities())) { - WebElement html = driver.findElement(By.tagName("html")); - // reset zoom level back to 100% - html.sendKeys(Keys.chord(Keys.CONTROL, "0")); - } - } - - private void openDebugExamineComponentHierarchyTab() { - WebElement button = findElement(By - .xpath("//button[@title='Examine component hierarchy']")); - // can't use 'click()' with zoom levels other than 100% - button.sendKeys(Keys.chord(Keys.SPACE)); - } - - private void clickDebugCheckLayoutsForPotentialProblems() { - WebElement button = findElement(By - .xpath("//button[@title='Check layouts for potential problems']")); - - button.sendKeys(Keys.chord(Keys.SPACE)); - } - - private WebElement findLayoutAnalyzePassElement() { - try { - return findElement(By - .xpath("//div[text()='Layouts analyzed, no top level problems']")); - } catch (NoSuchElementException e) { - return null; - } - } - - private WebElement findLayoutAnalyzeAbortedElement() { - try { - return findElement(By - .xpath("//div[text()='Aborting layout after 100 passess']")); - } catch (NoSuchElementException e) { - return null; - } - } -} |