ericmckean / wave-protocol

Automatically exported from code.google.com/p/wave-protocol
0 stars 0 forks source link

Shniy when clicking on a blip created with Data API #262

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The exceptions cannot be reproduced reliably. However it happens when someone 
clicks in the official WIAb web client on a blip created with Data API 
(micro-box) in order to mark it read. The exceptions happens only if the blip 
is in unread state.

What steps will reproduce the problem?
1. Create a blip in some wave using data api (micro-box)
2. Login into official WIAB web client as other user. The blip will appear as 
unread. Click on the blip to mark it as read.

Expected: The blip is marked as read.
Actual: Shiny

Stack trace:

Token:  1303153987392
 One or more exceptions caught, see full set in UmbrellaException#getCauses
  Unknown.$fillInStackTrace (StackTraceCreator.java:147)
  Unknown.fillInStackTrace (StackTraceCreator.java:387)
  Unknown.RuntimeException_2 (Throwable.java:46)
  Unknown.UmbrellaException_0 (com.google.gwt.dev.jjs.ast.JProgram:0)
  Unknown.$doFire (SimpleEventBus.java:214)
  Unknown.$fireEvent_0 (SimpleEventBus.java:103)
  Unknown.$fireEvent (HandlerManager.java:101)
  Unknown.$fireEvent_1 (Widget.java:103)
  Unknown.fireNativeEvent (DomEvent.java:116)
  Unknown.$onBrowserEvent (Widget.java:151)
  Unknown.onBrowserEvent (Widget.java:137)
  Unknown.dispatchEvent_1 (DOM.java:1264)
  Unknown.anonymous (DOMImplStandard.java:187)
  Unknown.entry0 (Impl.java:214)
  Unknown.anonymous (Impl.java:57)
Caused by: Token:  1303153987397
 null
  Unknown.$fillInStackTrace (StackTraceCreator.java:147)
  Unknown.fillInStackTrace (StackTraceCreator.java:387)
  Unknown.IllegalArgumentException_0 (Throwable.java:46)
  Unknown.checkArgument (Preconditions.java:72)
  Unknown.$asRootThread_1 (FullStructure.java:694)
  Unknown.$getRootThreadView (ModelAsViewProviderImpl.java:74)
  Unknown.$apply_25 (LiveSupplementRenderer.java:79)
  Unknown.apply_75 (LiveSupplementRenderer.java:74)
  Unknown.$eachInner_0 (JsIdentitySet.java:89)
  Unknown.each_0 (JsIdentitySet.java:63)
  Unknown.$notifyListeners_1 (ThreadReadStateMonitorImpl.java:654)
  Unknown.$countDownEvent (ThreadReadStateMonitorImpl.java:631)
  Unknown.onMaybeBlipReadChanged_1 (ThreadReadStateMonitorImpl.java:578)
  Unknown.$triggerOnMaybeBlipReadChanged (LiveSupplementedWaveImpl.java:213)
  Unknown.$triggerOnMaybeBlipReadChanged_0 (LiveSupplementedWaveImpl.java:284)
  Unknown.onLastReadBlipVersionChanged (LiveSupplementedWaveImpl.java:78)
  Unknown.$triggerOnLastReadBlipVersionChanged (WaveletBasedSupplement.java:674)
  Unknown.$onEntrySet_0 (Integer.java:320)
  Unknown.onEntrySet_0 (DocumentBasedWaveletReadState.java:90)
  Unknown.$triggerOnEntryChanged (AbstractDocumentBasedMap.java:369)
  Unknown.$put_8 (AbstractDocumentBasedMap.java:192)
  Unknown.$setLastReadBlipVersion_0 (DocumentBasedWaveletReadState.java:120)
  Unknown.$markBlipAsRead (SupplementImpl.java:117)
  Unknown.markAsRead_1 (SupplementedWaveImpl.java:304)
  Unknown.markAsRead (SupplementedWaveWrapper.java:50)
  Unknown.$startAutoReading (LocalSupplementedWaveImpl.java:177)
  Unknown.$startReading (LocalSupplementedWaveImpl.java:156)
  Unknown.onFocusMoved_0 (Reader.java:84)
  Unknown.$fireOnFocusMoved (FocusFramePresenter.java:188)
  Unknown.$focus_3 (FocusFramePresenter.java:156)
  Unknown.$focusWithoutScroll (FocusFramePresenter.java:113)
  Unknown.onMouseDown_1 (FocusFrameController.java:70)
  Unknown.$dispatch_16 (EventDispatcherPanel.java:271)
  Unknown.dispatch_21 (EventDispatcherPanel.java:270)
  Unknown.$dispatch_14 (EventDispatcherPanel.java:175)
  Unknown.onMouseDown (EventDispatcherPanel.java:276)
  Unknown.dispatch_6 (MouseDownEvent.java:53)
  Unknown.$doFire (SimpleEventBus.java:204)
  Unknown.$fireEvent_0 (SimpleEventBus.java:103)
  Unknown.$fireEvent (HandlerManager.java:101)
  Unknown.$fireEvent_1 (Widget.java:103)
  Unknown.fireNativeEvent (DomEvent.java:116)
  Unknown.$onBrowserEvent (Widget.java:151)
  Unknown.onBrowserEvent (Widget.java:137)
  Unknown.dispatchEvent_1 (DOM.java:1264)
  Unknown.anonymous (DOMImplStandard.java:187)
  Unknown.entry0 (Impl.java:214)
  Unknown.anonymous (Impl.java:57)

Please use labels and text to provide additional information.

Original issue reported on code.google.com by vega113 on 18 Apr 2011 at 8:29

GoogleCodeExporter commented 9 years ago
Hi Michael
This issue seems to be connected with inline replies. Can you please take a 
look at this?

Original comment by vega113 on 18 Apr 2011 at 8:31

GoogleCodeExporter commented 9 years ago
Example blip: wave://waveinabox.net/w+2-W46qg1MnA/~/conv+root/b+1njtdj3ecxjbiA

Original comment by vega113 on 18 Apr 2011 at 8:32

GoogleCodeExporter commented 9 years ago
I should have time to look at this issue tomorrow (4/19).

Original comment by michael.macfadden on 18 Apr 2011 at 8:37

GoogleCodeExporter commented 9 years ago
Is that blip a blip in the root thread, or a blip in an inline thread (i.e., an 
inline reply)?

If it is the latter, it looks to me like it's a bug in how the data API adds 
inline replies, because the conversation model thinks that that blip's thread 
is a root thread, not an inline thread.  Threads are distinguished as inline or 
not from the presence of an 'inline=true' attribute in the conversation 
manifest.

I know nothing about the data api, and I had a quick look at wave/api/*, and I 
couldn't find how blip additions are translated into conversation-model method 
calls.  Yuri, do you know that code at all?

Original comment by hearn...@google.com on 19 Apr 2011 at 10:05

GoogleCodeExporter commented 9 years ago
Actually, there is a much simpler fix.  In LiveSupplementRenderer:

         ThreadView threadUi = null;
-        if (thread.isInline()) {
+        if (thread.getParentBlip() != null) {
           threadUi = views.getInlineThreadView(thread);

The 'inline' attribute of a thread is no longer meaningful, since we don't have 
non-inline replies in WIAB.

Original comment by hearn...@google.com on 19 Apr 2011 at 10:08

GoogleCodeExporter commented 9 years ago
Thanks for you comment David. 
I think I know the code, so I ll try to investigate this direction.

Original comment by vega113 on 19 Apr 2011 at 10:09

GoogleCodeExporter commented 9 years ago
I agree with Dave's analysis in comment #5.  Looking at the issue, we are 
incorrectly treading an inline reply as a root thread.  When the 
ModelAsViewProviderImpl class tries to look up the element as a root thread, it 
can't find it, because it's not actually a root thread.  This then passes the 
view over to the FullStructure class as null which ultimately violates the 
precondition in that classes asRootThread method.

As Dave points out we should really clean up the inline attribute to avoid 
confusion, but for the moment if a thread has a parent blip, then it is an 
inline thread, otherwise its a root thread.  I would make the above change, but 
add a comment to that effect.

Original comment by michael.macfadden on 19 Apr 2011 at 3:31

GoogleCodeExporter commented 9 years ago
I have submitted an alternative patch for this here as per the above comments.

http://codereview.waveprotocol.org/574002/show

Original comment by michael.macfadden on 19 Apr 2011 at 3:56

GoogleCodeExporter commented 9 years ago
fixed by 618050651f4d

Original comment by vega113 on 21 Apr 2011 at 7:46