Cherry picking bug fixes from trunk into release branch: 991801, 989801, 995801, 988801 git-svn-id: https://google-web-toolkit.googlecode.com/svn/releases/2.1@9055 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/samples/dynatablerf/build.xml b/samples/dynatablerf/build.xml index 2848b88..25d3a2d 100755 --- a/samples/dynatablerf/build.xml +++ b/samples/dynatablerf/build.xml
@@ -13,5 +13,6 @@ <include name="apache/log4j/log4j-1.2.16.jar" /> <include name="slf4j/slf4j-api/slf4j-api-1.6.1.jar" /> <include name="slf4j/slf4j-log4j12/slf4j-log4j12-1.6.1.jar" /> + <include name="javax/xml/bind/jaxb-api-2.1.jar" /> </fileset> </project>
diff --git a/user/src/com/google/gwt/activity/shared/ActivityManager.java b/user/src/com/google/gwt/activity/shared/ActivityManager.java index 30626bf..2f044e5 100644 --- a/user/src/com/google/gwt/activity/shared/ActivityManager.java +++ b/user/src/com/google/gwt/activity/shared/ActivityManager.java
@@ -54,7 +54,7 @@ public void setWidget(IsWidget view) { if (this.activity == ActivityManager.this.currentActivity) { startingNext = false; - display.setWidget(view); + showWidget(view); } } } @@ -94,11 +94,17 @@ /** * Deactive the current activity, find the next one from our ActivityMapper, * and start it. - * + * <p> + * The current activity's widget will be hidden immediately, which can cause + * flicker if the next activity provides its widget asynchronously. That can + * be minimized by decent caching. Perenially slow activities might mitigate + * this by providing a widget immediately, with some kind of "loading" + * treatment. + * * @see PlaceChangeEvent.Handler#onPlaceChange(PlaceChangeEvent) */ public void onPlaceChange(PlaceChangeEvent event) { - Activity nextActivity = mapper.getActivity(event.getNewPlace()); + Activity nextActivity = getNextActivity(event); Throwable caughtOnStop = null; Throwable caughtOnStart = null; @@ -118,11 +124,7 @@ currentActivity = NULL_ACTIVITY; startingNext = false; } else if (!currentActivity.equals(NULL_ACTIVITY)) { - /* - * TODO until caching is in place, relying on stopped activities to be - * good citizens to reduce flicker. This makes me very nervous. - */ - // display.showActivityWidget(null); + showWidget(null); /* * Kill off the activity's handlers, so it doesn't have to worry about @@ -146,7 +148,7 @@ currentActivity = nextActivity; if (currentActivity.equals(NULL_ACTIVITY)) { - display.setWidget(null); + showWidget(null); return; } @@ -155,7 +157,7 @@ /* * Now start the thing. Wrap the actual display with a per-call instance * that protects the display from canceled or stopped activities, and which - * maintain our startingNext state. + * maintains our startingNext state. */ try { currentActivity.start(new ProtectedDisplay(currentActivity), @@ -179,7 +181,7 @@ /** * Reject the place change if the current activity is not willing to stop. - * + * * @see PlaceChangeRequestEvent.Handler#onPlaceChangeRequest(PlaceChangeRequestEvent) */ public void onPlaceChangeRequest(PlaceChangeRequestEvent event) { @@ -195,7 +197,7 @@ * If you are disposing of an ActivityManager, it is important to call * setDisplay(null) to get it to deregister from the event bus, so that it can * be garbage collected. - * + * * @param display */ public void setDisplay(AcceptsOneWidget display) { @@ -207,6 +209,24 @@ } } + private Activity getNextActivity(PlaceChangeEvent event) { + if (display == null) { + /* + * Display may have been nulled during PlaceChangeEvent dispatch. Don't + * bother the mapper, just return a null to ensure we shut down the + * current activity + */ + return null; + } + return mapper.getActivity(event.getNewPlace()); + } + + private void showWidget(IsWidget view) { + if (display != null) { + display.setWidget(view); + } + } + private void updateHandlers(boolean activate) { if (activate) { final HandlerRegistration placeReg = eventBus.addHandler(
diff --git a/user/src/com/google/gwt/activity/shared/ActivityMapper.java b/user/src/com/google/gwt/activity/shared/ActivityMapper.java index d7fce27..87a85c9 100644 --- a/user/src/com/google/gwt/activity/shared/ActivityMapper.java +++ b/user/src/com/google/gwt/activity/shared/ActivityMapper.java
@@ -27,5 +27,10 @@ * an {@link ActivityManager}. */ public interface ActivityMapper { + /** + * Returns the activity to run for the given {@link Place}, or null. + * + * @param place a Place object + */ Activity getActivity(Place place); }
diff --git a/user/src/com/google/gwt/logging/client/LoggingPopup.java b/user/src/com/google/gwt/logging/client/LoggingPopup.java index 0ddc1d3..1bb817c 100644 --- a/user/src/com/google/gwt/logging/client/LoggingPopup.java +++ b/user/src/com/google/gwt/logging/client/LoggingPopup.java
@@ -138,7 +138,7 @@ super(false, false); VerticalPanel mainPanel = new VerticalPanel(); mainPanel.setBorderWidth(1); - mainPanel.getElement().setAttribute("style", "background-color:white"); + mainPanel.getElement().getStyle().setBackgroundColor("white"); final HTML titleBar = new HTML("<center><b>Logging</b></center>"); mainPanel.add(titleBar); @@ -184,12 +184,12 @@ @Override public void add(Widget w) { logArea.add(w); + scrollPanel.setScrollPosition(scrollPanel.getElement().getScrollHeight()); } @Override public void setWidget(Widget w) { logArea.clear(); - logArea.add(w); + add(w); } - }
diff --git a/user/src/com/google/gwt/user/cellview/CellView.gwt.xml b/user/src/com/google/gwt/user/cellview/CellView.gwt.xml index ad0cbe0..22a4326 100644 --- a/user/src/com/google/gwt/user/cellview/CellView.gwt.xml +++ b/user/src/com/google/gwt/user/cellview/CellView.gwt.xml
@@ -24,7 +24,6 @@ <replace-with class="com.google.gwt.user.cellview.client.CellBasedWidgetImplStandard"> <when-type-is class="com.google.gwt.user.cellview.client.CellBasedWidgetImpl"/> <any> - <when-property-is name="user.agent" value="safari"/> <when-property-is name="user.agent" value="opera"/> <when-property-is name="user.agent" value="gecko"/> <when-property-is name="user.agent" value="gecko1_8"/> @@ -40,6 +39,14 @@ </any> </replace-with> + <!-- Safari-specific CellBasedWidgetImpl implementation. --> + <replace-with class="com.google.gwt.user.cellview.client.CellBasedWidgetImplSafari"> + <when-type-is class="com.google.gwt.user.cellview.client.CellBasedWidgetImpl"/> + <any> + <when-property-is name="user.agent" value="safari"/> + </any> + </replace-with> + <!-- IE-specific CellTable implementation. --> <replace-with class="com.google.gwt.user.cellview.client.CellTable.ImplTrident"> <when-type-is class="com.google.gwt.user.cellview.client.CellTable.Impl"/>
diff --git a/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java b/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java new file mode 100644 index 0000000..c24d875 --- /dev/null +++ b/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java
@@ -0,0 +1,31 @@ +/* + * Copyright 2010 Google Inc. + * + * 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.google.gwt.user.cellview.client; + +import com.google.gwt.core.client.Scheduler; +import com.google.gwt.core.client.Scheduler.ScheduledCommand; + +/** + * Webkit specified Impl used by cell based widgets. + */ +public class CellBasedWidgetImplSafari extends CellBasedWidgetImplStandard { + + @Override + public void resetFocus(ScheduledCommand command) { + // Webkit will not focus an element that was created in this event loop. + Scheduler.get().scheduleDeferred(command); + } +}
diff --git a/user/src/com/google/gwt/user/cellview/client/CellBrowser.css b/user/src/com/google/gwt/user/cellview/client/CellBrowser.css index 6428be8..8739e95 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellBrowser.css +++ b/user/src/com/google/gwt/user/cellview/client/CellBrowser.css
@@ -40,6 +40,7 @@ @sprite .cellBrowserSelectedItem { gwt-image: 'cellBrowserSelectedBackground'; background-color: #628cd5; + background-repeat: repeat-x; color: white; height: auto; overflow: hidden; @@ -48,6 +49,7 @@ @sprite .cellBrowserOpenItem { gwt-image: 'cellBrowserOpenBackground'; background-color: #7b7b7b; + background-repeat: repeat-x; color: white; height: auto; overflow: hidden;
diff --git a/user/src/com/google/gwt/user/cellview/client/CellBrowser.java b/user/src/com/google/gwt/user/cellview/client/CellBrowser.java index c98075e..64fabc2 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellBrowser.java +++ b/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
@@ -101,14 +101,16 @@ /** * The background used for open items. */ - @ImageOptions(repeatStyle = RepeatStyle.Horizontal, flipRtl = true) + // Use RepeatStyle.BOTH to ensure that we do not bundle the image. + @ImageOptions(repeatStyle = RepeatStyle.Both, flipRtl = true) ImageResource cellBrowserOpenBackground(); /** * The background used for selected items. */ + // Use RepeatStyle.BOTH to ensure that we do not bundle the image. @Source("cellTreeSelectedBackground.png") - @ImageOptions(repeatStyle = RepeatStyle.Horizontal, flipRtl = true) + @ImageOptions(repeatStyle = RepeatStyle.Both, flipRtl = true) ImageResource cellBrowserSelectedBackground(); /**
diff --git a/user/src/com/google/gwt/user/cellview/client/CellList.java b/user/src/com/google/gwt/user/cellview/client/CellList.java index 40ac250..fbfeaeb 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellList.java +++ b/user/src/com/google/gwt/user/cellview/client/CellList.java
@@ -378,6 +378,9 @@ // If the event causes us to page, then the index will be out of bounds. return; } + + // Get the cell parent before doing selection in case the list is redrawn. + Element cellParent = getCellParent(target); T value = getDisplayedItem(indexOnPage); if (isMouseDown && !cell.handlesSelection()) { doSelection(event, value, indexOnPage); @@ -390,7 +393,7 @@ } // Fire the event to the cell if the list has not been refreshed. - fireEventToCell(event, getCellParent(target), value); + fireEventToCell(event, cellParent, value); } }
diff --git a/user/src/com/google/gwt/user/cellview/client/CellTable.java b/user/src/com/google/gwt/user/cellview/client/CellTable.java index 17fc161..302185a 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellTable.java +++ b/user/src/com/google/gwt/user/cellview/client/CellTable.java
@@ -905,7 +905,8 @@ boolean isMouseDown = "mousedown".equals(eventType); int row = tr.getSectionRowIndex(); if ("mouseover".equals(eventType)) { - if (hoveringRow != null) { + // Unstyle the old row if it is still part of the table. + if (hoveringRow != null && tbody.isOrHasChild(hoveringRow)) { setRowStyleName(hoveringRow, style.cellTableHoveredRow(), style.cellTableHoveredRowCell(), false); }
diff --git a/user/src/com/google/gwt/user/cellview/client/CellTree.java b/user/src/com/google/gwt/user/cellview/client/CellTree.java index 78273fc..0c1333a 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellTree.java +++ b/user/src/com/google/gwt/user/cellview/client/CellTree.java
@@ -18,6 +18,7 @@ import com.google.gwt.animation.client.Animation; import com.google.gwt.core.client.GWT; import com.google.gwt.core.client.Scheduler; +import com.google.gwt.core.client.Scheduler.ScheduledCommand; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.Style.Display; import com.google.gwt.dom.client.Style.Position; @@ -495,6 +496,11 @@ private boolean isAnimationEnabled; /** + * The deferred command used to keyboard select a node. + */ + private ScheduledCommand keyboardSelectCommand; + + /** * The {@link CellTreeNodeView} whose children are currently being selected * using the keyboard. */ @@ -667,7 +673,7 @@ collectElementChain(chain, getElement(), target); boolean isMouseDown = "mousedown".equals(eventType); - CellTreeNodeView<?> nodeView = findItemByChain(chain, 0, rootNode); + final CellTreeNodeView<?> nodeView = findItemByChain(chain, 0, rootNode); if (nodeView != null && nodeView != rootNode) { if (isMouseDown) { Element showMoreElem = nodeView.getShowMoreElement(); @@ -683,11 +689,21 @@ } // Forward the event to the cell - if (nodeView.getCellParent().isOrHasChild(target)) { + if (nodeView.getSelectionElement().isOrHasChild(target)) { // Move the keyboard focus to the clicked item. if ("focus".equals(eventType) || isMouseDown) { - isFocused = true; - keyboardSelect(nodeView, false); + // Wait until any pending blur event has fired. + final boolean targetsCellParent = nodeView.getCellParent().isOrHasChild(target); + keyboardSelectCommand = new ScheduledCommand() { + public void execute() { + if (keyboardSelectCommand == this && !nodeView.isDestroyed()) { + isFocused = true; + keyboardSelectCommand = null; + keyboardSelect(nodeView, !targetsCellParent); + } + } + }; + Scheduler.get().scheduleDeferred(keyboardSelectCommand); } nodeView.fireEventToCell(event); @@ -858,7 +874,8 @@ void resetFocus() { CellBasedWidgetImpl.get().resetFocus(new Scheduler.ScheduledCommand() { public void execute() { - if (isFocused && !keyboardSelectedNode.resetFocusOnCell()) { + if (isFocused && !keyboardSelectedNode.isDestroyed() + && !keyboardSelectedNode.resetFocusOnCell()) { keyboardSelectedNode.setKeyboardSelected(true, true); } }
diff --git a/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java b/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java index 634efe1..2afe4b7 100644 --- a/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java +++ b/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
@@ -974,6 +974,9 @@ } // Forward the event to the cell. + if (!cellParent.isOrHasChild(Element.as(event.getEventTarget()))) { + return; + } Set<String> consumedEvents = parentCell.getConsumedEvents(); if (consumedEvents != null && consumedEvents.contains(eventType)) { boolean cellWasEditing = parentCell.isEditing(cellParent, value, key); @@ -1007,6 +1010,17 @@ } /** + * Returns the element that selection styles are applied to. The element + * includes the open/close image and the rendered value and spans the width of + * the tree. + * + * @return the selection element + */ + protected Element getSelectionElement() { + return getSelectionElement(getElement()); + } + + /** * Returns the key for the value of this node using the parent's * implementation of NodeInfo.getKey(). */ @@ -1039,7 +1053,7 @@ animationFrame = Document.get().createDivElement(); animationFrame.getStyle().setPosition(Position.RELATIVE); animationFrame.getStyle().setOverflow(Overflow.HIDDEN); - animationFrame.setId("animFrame"); + animationFrame.getStyle().setDisplay(Display.NONE); getElement().appendChild(animationFrame); } return animationFrame;
diff --git a/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java b/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java index d5d47d0..0f25f8b 100644 --- a/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java +++ b/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java
@@ -189,11 +189,7 @@ assertNull(asyncActivity2.display); eventBus.fireEvent(new PlaceChangeEvent(place2)); - /* - * TODO until caching is in place, relying on stopped activities to be good - * citizens to reduce flicker. This makes me very nervous. - */ - // assertNull(realDisplay.widget); + assertNull(realDisplay.widget); assertFalse(asyncActivity1.canceled); assertTrue(asyncActivity1.stopped); assertFalse(asyncActivity2.stopped); @@ -354,6 +350,60 @@ assertNotNull(activity2.display); assertEquals(0, eventBus.getCount(Event.TYPE)); } + + /** + * http://code.google.com/p/google-web-toolkit/issues/detail?id=5375 + */ + public void testNullDisplayOnPlaceChange() { + manager.setDisplay(realDisplay); + + // Start an activity + manager.onPlaceChange(new PlaceChangeEvent(place1)); + + /* + * Now we're going to place2. During PlaceChangeEvent dispatch, + * someone kills the manager's display. + */ + manager.setDisplay(null); + + // Now the place change event reaches the manager + manager.onPlaceChange(new PlaceChangeEvent(place2)); + + assertNull(activity2.display); + assertTrue(activity1.stopped); + } + + public void testNullDisplayBeforeAsyncStart() { + final AsyncActivity asyncActivity1 = new AsyncActivity(new MyView()); + final AsyncActivity asyncActivity2 = new AsyncActivity(new MyView()); + + ActivityMapper map = new ActivityMapper() { + public Activity getActivity(Place place) { + if (place.equals(place1)) { + return asyncActivity1; + } + if (place.equals(place2)) { + return asyncActivity2; + } + + return null; + } + }; + + manager = new ActivityManager(map, eventBus); + manager.setDisplay(realDisplay); + + // Start an activity + manager.onPlaceChange(new PlaceChangeEvent(place1)); + + // Kill the manager + manager.setDisplay(null); + + // The activity is ready to play + asyncActivity1.finish(); + + // Ta da, no NPE + } public void testRejected() { manager.setDisplay(realDisplay);