diff options
4 files changed, 622 insertions, 2 deletions
diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index c56a2a8772..bb431bf132 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -991,6 +991,12 @@ public class VScrollTable extends FlowPanel implements HasWidgets, if (scrollBody != null) { scrollBody.removeFromParent(); } + + // Without this call the scroll position is messed up in IE even after + // the lazy scroller has set the scroll position to the first visible + // item + scrollBodyPanel.getScrollPosition(); + scrollBody = createScrollBody(); scrollBody.renderInitialRows(rowData, uidl.getIntAttribute("firstrow"), @@ -1121,7 +1127,28 @@ public class VScrollTable extends FlowPanel implements HasWidgets, } } + private boolean lazyScrollerIsActive; + + private void disableLazyScroller() { + lazyScrollerIsActive = false; + scrollBodyPanel.getElement().getStyle().clearOverflowX(); + scrollBodyPanel.getElement().getStyle().clearOverflowY(); + } + + private void enableLazyScroller() { + Scheduler.get().scheduleDeferred(lazyScroller); + lazyScrollerIsActive = true; + // prevent scrolling to jump in IE11 + scrollBodyPanel.getElement().getStyle().setOverflowX(Overflow.HIDDEN); + scrollBodyPanel.getElement().getStyle().setOverflowY(Overflow.HIDDEN); + } + + private boolean isLazyScrollerActive() { + return lazyScrollerIsActive; + } + private ScheduledCommand lazyScroller = new ScheduledCommand() { + @Override public void execute() { if (firstvisible > 0) { @@ -1134,6 +1161,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, .setScrollPosition(measureRowHeightOffset(firstvisible)); } } + disableLazyScroller(); } }; @@ -1152,7 +1180,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, // Only scroll if the first visible changes from the server side. // Else we might unintentionally scroll even when the scroll // position has not changed. - Scheduler.get().scheduleDeferred(lazyScroller); + enableLazyScroller(); } } @@ -2172,7 +2200,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, isNewBody = false; if (firstvisible > 0) { - Scheduler.get().scheduleDeferred(lazyScroller); + enableLazyScroller(); } if (enabled) { @@ -6871,6 +6899,12 @@ public class VScrollTable extends FlowPanel implements HasWidgets, @Override public void onScroll(ScrollEvent event) { + // Do not handle scroll events while there is scroll initiated from + // server side which is not yet executed (#11454) + if (isLazyScrollerActive()) { + return; + } + scrollLeft = scrollBodyPanel.getElement().getScrollLeft(); scrollTop = scrollBodyPanel.getScrollPosition(); /* diff --git a/uitest/src/com/vaadin/tests/components/table/ExpandingContainer.java b/uitest/src/com/vaadin/tests/components/table/ExpandingContainer.java new file mode 100644 index 0000000000..829c29b95b --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/ExpandingContainer.java @@ -0,0 +1,429 @@ +package com.vaadin.tests.components.table; + +import java.util.AbstractList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.logging.Logger; + +import com.vaadin.data.Container; +import com.vaadin.data.Item; +import com.vaadin.data.Property; +import com.vaadin.data.util.AbstractContainer; +import com.vaadin.data.util.BeanItem; +import com.vaadin.server.VaadinSession; +import com.vaadin.ui.Component; +import com.vaadin.ui.Label; + +@SuppressWarnings("serial") +public class ExpandingContainer extends AbstractContainer implements + Container.Ordered, Container.Indexed, Container.ItemSetChangeNotifier { + + public static final List<String> PROPERTY_IDS = Arrays.asList("id", + "column1", "column2"); + + private final Label sizeLabel; + private final Logger log = Logger.getLogger(this.getClass().getName()); + + private int currentSize = 300; + + private boolean loggingEnabled; + + public ExpandingContainer(Label sizeLabel) { + this.sizeLabel = sizeLabel; + updateLabel(); + } + + private void log(String message) { + if (loggingEnabled) { + log.info(message); + } + } + + // Expand container if we scroll past 85% + public int checkExpand(int index) { + log("checkExpand(" + index + ")"); + if (index >= currentSize * 0.85) { + final int oldsize = currentSize; + currentSize = (int) (oldsize * 1.3333); + log("*** getSizeWithHint(" + index + "): went past 85% of size=" + + oldsize + ", new size=" + currentSize); + updateLabel(); + } + return currentSize; + }; + + @Override + public void fireItemSetChange() { + super.fireItemSetChange(); + } + + private void updateLabel() { + sizeLabel.setValue("Container size: " + currentSize); + } + + public void triggerItemSetChange() { + log("*** triggerItemSetChange(): scheduling item set change event"); + final VaadinSession session = VaadinSession.getCurrent(); + new Thread() { + @Override + public void run() { + ExpandingContainer.this.invoke(session, new Runnable() { + @Override + public void run() { + log("*** Firing item set change event"); + ExpandingContainer.this.fireItemSetChange(); + } + }); + } + }.start(); + } + + private void invoke(VaadinSession session, Runnable action) { + session.lock(); + VaadinSession previousSession = VaadinSession.getCurrent(); + VaadinSession.setCurrent(session); + try { + action.run(); + } finally { + session.unlock(); + VaadinSession.setCurrent(previousSession); + } + } + + // Container + + @Override + public BeanItem<MyBean> getItem(Object itemId) { + if (!(itemId instanceof Integer)) { + return null; + } + final int index = ((Integer) itemId).intValue(); + return new BeanItem<MyBean>(new MyBean(index)); + } + + @Override + public Collection<Integer> getItemIds() { + return new IntList(size()); + } + + @Override + public List<String> getContainerPropertyIds() { + return PROPERTY_IDS; + } + + @Override + @SuppressWarnings("rawtypes") + public Property/* <?> */getContainerProperty(Object itemId, + Object propertyId) { + BeanItem<MyBean> item = getItem(itemId); + return item != null ? item.getItemProperty(propertyId) : null; + } + + @Override + public Class<?> getType(Object propertyId) { + return Component.class; + } + + @Override + public int size() { + return currentSize; + } + + @Override + public boolean containsId(Object itemId) { + if (!(itemId instanceof Integer)) { + return false; + } + int index = ((Integer) itemId).intValue(); + checkExpand(index); + return index >= 0 && index < currentSize; + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public Item addItem(Object itemId) { + throw new UnsupportedOperationException(); + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public Item addItem() { + throw new UnsupportedOperationException(); + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public boolean removeItem(Object itemId) { + throw new UnsupportedOperationException(); + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public boolean addContainerProperty(Object propertyId, Class<?> type, + Object defaultValue) { + throw new UnsupportedOperationException(); + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public boolean removeContainerProperty(Object propertyId) { + throw new UnsupportedOperationException(); + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public boolean removeAllItems() { + throw new UnsupportedOperationException(); + } + + // Container.Indexed + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public Object addItemAt(int index) { + throw new UnsupportedOperationException(); + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public Item addItemAt(int index, Object newItemId) { + throw new UnsupportedOperationException(); + } + + @Override + public Integer getIdByIndex(int index) { + if (index < 0) { + throw new IndexOutOfBoundsException("index < " + index); + } + final int size = currentSize; + if (index >= size) { + throw new IndexOutOfBoundsException("index=" + index + " but size=" + + size); + } + checkExpand(index); + return index; + } + + @Override + public List<Integer> getItemIds(int startIndex, int numberOfItems) { + if (numberOfItems < 0) { + throw new IllegalArgumentException("numberOfItems < 0"); + } + final int size = currentSize; + checkExpand(startIndex); + if (startIndex < 0 || startIndex > size) { + throw new IndexOutOfBoundsException("startIndex=" + startIndex + + " but size=" + size); + } + if (startIndex + numberOfItems > size) { + numberOfItems = size - startIndex; + } + return new IntList(startIndex, numberOfItems); + } + + @Override + public int indexOfId(Object itemId) { + if (!(itemId instanceof Integer)) { + return -1; + } + final int index = ((Integer) itemId).intValue(); + checkExpand(index); + if (index < 0 || index >= currentSize) { + return -1; + } + return index; + } + + // Container.Ordered + + @Override + public Integer nextItemId(Object itemId) { + if (!(itemId instanceof Integer)) { + return null; + } + int index = ((Integer) itemId).intValue(); + checkExpand(index); + if (index < 0 || index + 1 >= currentSize) { + return null; + } + return index + 1; + } + + @Override + public Integer prevItemId(Object itemId) { + if (!(itemId instanceof Integer)) { + return null; + } + int index = ((Integer) itemId).intValue(); + checkExpand(index); + if (index - 1 < 0 || index >= currentSize) { + return null; + } + return index - 1; + } + + @Override + public Integer firstItemId() { + return currentSize == 0 ? null : 0; + } + + @Override + public Integer lastItemId() { + final int size = currentSize; + return size == 0 ? null : size - 1; + } + + @Override + public boolean isFirstId(Object itemId) { + if (!(itemId instanceof Integer)) { + return false; + } + final int index = ((Integer) itemId).intValue(); + checkExpand(index); + final int size = currentSize; + return size > 0 && index == 0; + } + + @Override + public boolean isLastId(Object itemId) { + if (!(itemId instanceof Integer)) { + return false; + } + int index = ((Integer) itemId).intValue(); + checkExpand(index); + int size = currentSize; + return size > 0 && index == size - 1; + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public Item addItemAfter(Object previousItemId) { + throw new UnsupportedOperationException(); + } + + /** + * @throws UnsupportedOperationException + * always + */ + @Override + public Item addItemAfter(Object previousItemId, Object newItemId) { + throw new UnsupportedOperationException(); + } + + // Container.ItemSetChangeNotifier + + @Override + @SuppressWarnings("deprecation") + public void addListener(Container.ItemSetChangeListener listener) { + super.addListener(listener); + } + + @Override + public void addItemSetChangeListener( + Container.ItemSetChangeListener listener) { + super.addItemSetChangeListener(listener); + } + + @Override + @SuppressWarnings("deprecation") + public void removeListener(Container.ItemSetChangeListener listener) { + super.removeListener(listener); + } + + @Override + public void removeItemSetChangeListener( + Container.ItemSetChangeListener listener) { + super.removeItemSetChangeListener(listener); + } + + // IntList + + private static class IntList extends AbstractList<Integer> { + + private final int min; + private final int size; + + public IntList(int size) { + this(0, size); + } + + public IntList(int min, int size) { + if (size < 0) { + throw new IllegalArgumentException("size < 0"); + } + this.min = min; + this.size = size; + } + + @Override + public int size() { + return size; + } + + @Override + public Integer get(int index) { + if (index < 0 || index >= size) { + throw new IndexOutOfBoundsException(); + } + return min + index; + } + } + + // MyBean + public class MyBean { + + private final int index; + + public MyBean(int index) { + this.index = index; + } + + public String getId() { + return "ROW #" + index; + } + + public String getColumn1() { + return genText(); + } + + public String getColumn2() { + return genText(); + } + + private String genText() { + return "this is a line of text in row #" + index; + } + } + + public void logDetails(boolean enabled) { + loggingEnabled = enabled; + } +} diff --git a/uitest/src/com/vaadin/tests/components/table/ExpandingContainerVisibleRowRaceCondition.java b/uitest/src/com/vaadin/tests/components/table/ExpandingContainerVisibleRowRaceCondition.java new file mode 100644 index 0000000000..8ad2d7f9c7 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/ExpandingContainerVisibleRowRaceCondition.java @@ -0,0 +1,67 @@ +package com.vaadin.tests.components.table; + +import java.util.Map; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.ui.Label; +import com.vaadin.ui.Table; +import com.vaadin.ui.UI; +import com.vaadin.ui.VerticalLayout; + +@SuppressWarnings("serial") +public class ExpandingContainerVisibleRowRaceCondition extends UI { + + static final String TABLE = "table"; + + @Override + public void init(VaadinRequest request) { + final VerticalLayout rootLayout = new VerticalLayout(); + + rootLayout.setSpacing(true); + rootLayout.setSizeFull(); + rootLayout.setMargin(true); + + final Label sizeLabel = new Label(); + final ExpandingContainer container = new ExpandingContainer(sizeLabel); + container.logDetails(false); + + Table table = new Table(null, container) { + @Override + public void changeVariables(Object source, + Map<String, Object> variables) { + if (variables.containsKey("firstvisible")) { + int index = (Integer) variables.get("firstvisible"); + container.checkExpand(index); + } + if (variables.containsKey("reqfirstrow") + || variables.containsKey("reqrows")) { + try { + int index = ((Integer) variables + .get("lastToBeRendered")).intValue(); + container.checkExpand(index); + } catch (Exception e) { + // do nothing + } + } + super.changeVariables(source, variables); + } + }; + table.setId(TABLE); + table.setCacheRate(0); + table.setSizeFull(); + table.setVisibleColumns(ExpandingContainer.PROPERTY_IDS + .toArray(new String[ExpandingContainer.PROPERTY_IDS.size()])); + + table.setCurrentPageFirstItemIndex(120); + + rootLayout.addComponent(table); + rootLayout.setExpandRatio(table, 1); + + rootLayout.addComponent(sizeLabel); + + setContent(rootLayout); + + container.checkExpand(300); + } + +} diff --git a/uitest/src/com/vaadin/tests/components/table/ExpandingContainerVisibleRowRaceConditionTest.java b/uitest/src/com/vaadin/tests/components/table/ExpandingContainerVisibleRowRaceConditionTest.java new file mode 100644 index 0000000000..73dd7b1f81 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/ExpandingContainerVisibleRowRaceConditionTest.java @@ -0,0 +1,90 @@ +/* + * Copyright 2000-2013 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.table; + +import static com.vaadin.tests.components.table.ExpandingContainerVisibleRowRaceCondition.TABLE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.fail; + +import java.util.List; + +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class ExpandingContainerVisibleRowRaceConditionTest extends + MultiBrowserTest { + + private static final int ROW_HEIGHT = 20; + + @Test + public void testScrollingWorksWithoutJumpingWhenItemSetChangeOccurs() { + openTestURL(); + sleep(1000); + + WebElement table = vaadinElementById(TABLE); + assertFirstRowIdIs("ROW #120"); + + testBenchElement(table.findElement(By.className("v-scrollable"))) + .scroll(320 * ROW_HEIGHT); + sleep(1000); + + assertRowIdIsInThePage("ROW #330"); + assertScrollPositionIsNotVisible(); + } + + @Override + protected void sleep(int milliseconds) { + try { + super.sleep(milliseconds); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + + private void assertFirstRowIdIs(String expected) { + List<WebElement> cellsOfFirstColumn = getCellsOfFirstColumn(); + WebElement first = cellsOfFirstColumn.get(0); + assertEquals(expected, first.getText()); + } + + private void assertRowIdIsInThePage(String expected) { + List<WebElement> cellsOfFirstColumn = getCellsOfFirstColumn(); + for (WebElement rowId : cellsOfFirstColumn) { + if (expected.equals(rowId.getText())) { + return; + } + } + fail("Expected row was not found"); + } + + private void assertScrollPositionIsNotVisible() { + WebElement table = vaadinElementById(TABLE); + WebElement scrollPosition = table.findElement(By + .className("v-table-scrollposition")); + assertFalse(scrollPosition.isDisplayed()); + } + + private List<WebElement> getCellsOfFirstColumn() { + WebElement table = vaadinElementById(TABLE); + List<WebElement> firstCellOfRows = table.findElements(By + .cssSelector(".v-table-table tr > td")); + return firstCellOfRows; + } +} |