ericmckean / wave-protocol

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

Implement private reply feature. Starter project. #168

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This is a starter project to add the private-reply feature from Google Wave.

First, ensure that you are comfortable working in the Wave-In-A-Box development 
environment (see http://www.waveprotocol.org/code), and consider doing the 
sample tutorial.

Most of the code to handle existing private replies is already present in 
Wave-In-A-Box.  However, there is no UI action for creating them, so that code 
is not currently exercised.  This project adds a logical UI action for creating 
a private reply, in order to exercise that code path.  A separate project will 
add the physical UI controls for handling the appropriate gestures (drop-down 
menus etc.).

1) Piggy-back off the normal reply action.

All the edit-related UI actions are implemented in 
client/wavepanel/impl/edit/Actions.java.  First, hijack the existing reply 
action, and change it to insert a whole conversation object (a private reply) 
rather than just a regular reply thread.

The conversation model interprets a wave as a collection of Conversations.  
Some of these Conversations are anchored at blips of other Conversations.  
There is no conversation-model API for creating private replies directly.  
Instead, a private reply is formed by creating a new Conversation in the 
collection, and then setting its anchor to point at the appropriate blip.  The 
collection of conversations in a wave is exposed through the type 
model.conversation.ConversationView (the word 'view' is unfortunately 
overloaded: in the model code, it is a legacy name that just means a 
collection; in the client code, it means a UI object).

Replace the reply code in Actions.reply(BlipView) with the private-reply 
version:

-    ConversationBlip reply =
-        blip.appendInlineReplyThread(blip.getContent().size() - 1).appendBlip()
+    
+    Conversation conv = wave.createConversation();
+    ConversationBlip reply = conv.getRootThread().appendBlip();
+    conv.setAnchor(blip.getConversation().createAnchor(blip));    

This code adds a dependency on the conversation collection ('wave').

2) Inject the dependency.

Undercurrent follows a manual dependency-injection (DI) style.  In short, this 
just means that constructors are not public, and generally just set fields to 
parameters; no work, and no method calls or calls to other constructors.  Any 
logic required for construction is put in static factory methods.  This makes 
it easy to instantiate objects with mocks for testing.

Threading through the dependency on the whole conversation model requires the 
following additions (Eclipse can do many of these for you automatically).
* Add a 'private final ConversationView wave;' field to Actions;
* Add it to the Actions constructor, following the DI pattern;
* Add a 'ConverstionView wave' parameter to EditActionsBuilder.createAndInstall
* Update StageThree.DefaultProvider.createEditActions() with the local variable:

  ...
  ModelAsViewProvider views = stageTwo.getModelAsViewProvider();
+ ConversationView wave = stageTwo.getConversations();
  BlipQueueRenderer blipQueue = stageTwo.getBlipQueue();
  ...
- Actions actions =
    EditActionsBuilder.createAndInstall(panel, views, profiles, edit, blipQueue, focus);
+ Actions actions =
    EditActionsBuilder.createAndInstall(panel, views, wave, profiles, edit, blipQueue, focus);

This threading touches on two parts of Undercurrent: the layout of wave panel 
features, and the staged loading process.  First, Wave panel features are found 
in subpackages of wavepanel.impl (like edit, focus, reader, etc), and each such 
subpackage has a Builder for creating and installing that feature on demand. 
Second, related feature groups are bundled into 'stages' (StageZero, StageOne, 
...).  Editing features are bundled into StageThree.  Each stage has a provider 
class for creating and configuring each component of that stage.  Applications 
can override parts of these providers in order to customize or replace the 
various components in according to their specific needs.  The 
StageThree.DefaultProvider.createEditActions() method provides the default 
implementation and configuration of the wave-panel editing actions.

3) Test the feature in the wave panel harness

Start up the wave panel test harness, to try out this feature:
 $ ant waveharness_hosted
then visit the URL produced by the GWT code server (this is straight from the 
tutorial).  Once the code has been loaded and the page is ready, click reply on 
a blip.  Notice nothing happens in the UI, but there is an exception reported 
at the bottom of the GWT code server log (in the tab that started up for your 
browser session).  Unfortunately, there is not a clear indication that 
something went wrong - you just have to get used to checking the GWT code 
server tabs periodically for exceptions while testing.

The exception is an NPE in LiveConversationViewRenderer.onConversationAdded().

4. Fix the renderer bug.

In Undercurrent, live rendering is performed by renderers that observe models.  
The main renderer for a wave is the conversation renderer.  It has event 
callbacks for conversation events, and these are invoked as the conversation 
model(s) change.

The code is:

  public void onConversationAdded(ObservableConversation conversation) {
    BlipView container = viewOf(conversation.getAnchor().getBlip());
    if (container != null) {
      ...
    }
    observe(conversation);
  }

The bug is that this code assumes that any conversations that show up 
dynamically have anchors.  This is generally true, since the main way that 
conversations show up dynamically is because of private-reply addition, and 
private replies have anchors.  However, since the Wave platform has no 
transaction facility, and most event APIs in Wave are synchronous, event 
callbacks can often fire at inconvenient times (i.e., during the middle of a 
compound action, revealing the model in a transient intermediate state).  
Recall that the code to add the private reply created a new conversation first, 
*then* set its anchor.  This causes this onConversationAdded event to fire 
before the anchor is set.

A simple null check fixes this.  Note that the LiveConversationViewRenderer 
ensures that all conversations in view are being observed, and their events are 
handled by a ConversationUpdater.  The anchor being set after the conversation 
has been added is already handled by ConversationUpdater.onAnchorChanged().

- BlipView container = viewOf(conversation.getAnchor().getBlip());
- if (container != null) {
-   ConversationView conversationUi = container.insertConversationBefore(null, 
conversation);
- }
+ Anchor location = conversation.getAnchor();
+ if (location != null) {
+   BlipView container = viewOf(location.getBlip());
+   if (container != null) {
+     ...
+   }
+ }
  observe(conversation);

After saving this change, refresh the browser (you do not need to restart the 
GWT code server), and try to reply to a blip again.

5. Fix the other renderer bug.

Again, nothing happens in the UI, but there is an exception in the OOPHM log: 
an ISE in LiveConversationViewRenderer$ConversationUpdater.onBlipAdded().  
Again, this is a transient intermediate model state issue.  Since the new 
private reply won't be rendered until it is anchored somewhere to the 
conversation structure, there is no thread view for the new conversation's root 
thread.  The onBlipAdded event is firing due to the code in Actions.reply() 
adding a root blip to the new conversation, and it is trying to render that new 
blip and attach the rendering to the rendering of its containing thread.  Since 
it can not find that rendering (because it does not exist yet), it throws an 
ISE.

Since it is generally accepted that not all parts of the model are necessarily 
rendered at all times, throwing an ISE in that situation does not make sense.  
Just delete the throw statement (and similarly in onThreadAdded() in the same 
class.

Save the changes and refresh the browser.  Observe that creating private 
replies now works correctly in the harness.

6. Test concurrent behaviour in the full client.

The feature is now working in the isolated environment of the wave harness.  
The next step is to try it out in the full web client.  Build and run the Wave 
In A Box server, then launch the GWT code server for the full client:
 $ ant hosted_gwt
In two browser windows, visit the URL:
  http://<yourhostname>:9898/?gwt.codesvr=<yourhostname>:9997
(the URL is not produced for you anymore, like in the harness, because of how 
hostnames are set up for the Wave In a Box server).  If you get redirected 
around because of needing to login, ensure that once you've returned to the 
main page, that the ?gwt.codesvr=... URL parameter is put back.

Create a wave in one window, and open it in the other window.  Then create a 
private reply in one window, and observe it show up live in the other window.

Congratulations!  You've added the code to create private replies.
Now that you've verified that the code works, restore the original 
implementation of Actions.reply(BlipView) to a regular reply, but add a method 
Actions.replyPrivately(BlipView) to hold the private-reply creation code.  
Later, appropriate UI controls can be added to call that action.

Original issue reported on code.google.com by hearn...@google.com on 26 Nov 2010 at 6:51

GoogleCodeExporter commented 9 years ago

Original comment by hearn...@google.com on 26 Nov 2010 at 6:51

GoogleCodeExporter commented 9 years ago
Any news on this issue? Is anybody still trying to solve it? If not I'd like to

Original comment by glozan...@gmail.com on 11 Jan 2011 at 6:50

GoogleCodeExporter commented 9 years ago
I don't think anyone is working on it now. Go for it!

Original comment by ano...@google.com on 12 Jan 2011 at 3:11

GoogleCodeExporter commented 9 years ago
I had some issues completing this starter project. 

I started on revision 830, everything worked the way it was supposed to until 
step 6, when testing concurrent behavior. Running from hosted-gwt upon clicking 
"Reply" on one window didn't show anything on the other. There was no exception 
reported in hosted-gwt. After compiling code and doing the same thing I got a 
shiny that reported a TypeError. I tried to locate the error with no success.

This morning I hg pulled the new code and updated it and the method 
onAnchorChange specified in step 5, LiveViewConversationRenderer has been 
changed. I tried to follow around this step, but all I get is an 
UmbrellaException...

Any tips? Is there a way to solve this? Should this be left on hold?

Original comment by glozan...@gmail.com on 20 Jan 2011 at 8:36

GoogleCodeExporter commented 9 years ago
What is the exception you get?  An UmbrellaException typically wraps a few 
layers of causes.  Part of the value of a starter project is getting into the 
debugging process, and getting a little familiar with how that works with GWT 
and the WIAB code.

When you run with compiled code, if you use compile-gwt-dev rather than 
compile-gwt, you should get a meaningful stack trace in the shiny bar to locate 
that TypeError.

For step 6, the concurrent behaviour of private replies may not work properly 
right now, and a new communication protocol is in the works.  I wouldn't worry 
about it just yet.  If you get the other steps done, then that's fine.  
Especially if you can figure out what's causing that TypeError.

Original comment by hearn...@google.com on 27 Jan 2011 at 3:16

GoogleCodeExporter commented 9 years ago
After fixing I used compile-gwt-dev and the stack trace I get is something to 
do with 

SelectionW3CNative.java:200", this$static).setBaseAndExtent is not a function 
fileName: :

Full Stack trace:
One or more exceptions caught, see full set in UmbrellaException#getCauses
Unknown.$collect (JsArrayString.java:42)
Unknown.fillInStackTrace_2 (StackTraceCreator.java:147)
Unknown.fillInStackTrace_0 (StackTraceCreator.java:387)
Unknown.fillInStackTrace (Throwable.java:72)
Unknown.$$init_16 (Throwable.java:46)
Unknown.Throwable_2 (Throwable.java:56)
Unknown.Exception_2 (Exception.java:33)
Unknown.RuntimeException_2 (RuntimeException.java:33)
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.fireEvent_3 (Widget.java:101)
Unknown.fireNativeEvent (DomEvent.java:116)
Unknown.$onBrowserEvent (Widget.java:151)
Unknown.onBrowserEvent (Widget.java:137)
Unknown.dispatchEventImpl (DOM.java:1308)
Unknown.dispatchEvent_0 (DOM.java:1264)
Unknown.anonymous (DOMImplStandard.java:187)
Unknown.apply (Impl.java:168)
Unknown.entry0 (Impl.java:214)
Unknown.anonymous (Impl.java:57)
Caused by: (TypeError): ($location_0[stackIndex] = 
"SelectionW3CNative.java:200", this$static).setBaseAndExtent is not a function 
fileName: 
http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.html 
lineNumber: 19544 stack: $setBaseAndExtent([object Selection],[object 
HTMLDivElement],0,[object 
HTMLDivElement],0)@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652
E3A.cache.html:19544 $setAnchorAndFocus([object Selection],[object 
HTMLDivElement],0,[object 
HTMLDivElement],0)@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652
E3A.cache.html:19002 $set_8([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:18917 set_16([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:18971 set_13([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:18694 setCaret_5([object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:18703 $setCaret([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:8281 setCaret_0([object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:8430 $safelyRestoreSelection([object Object],[object 
Object],false)@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.
cache.html:7334 
focus_4(false)@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.
cache.html:7541 $startNewSession([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:21452 $startEditing_1([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:21437 $focusAndEdit([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:21046 $reply([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:21102 onMouseDown_3([object Object],[object 
HTMLSpanElement])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E
3A.cache.html:21862 $dispatch_23([object Object],[object 
HTMLSpanElement],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:45631 dispatch_22([object Object],[object HTMLSpanElement],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:45652 $dispatch_20([object Object],[object Object],[object 
HTMLSpanElement])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E
3A.cache.html:45441 onMouseDown([object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:45668 $dispatch_6([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:7592 dispatch_6([object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:7620 $doFire([object Object],[object 
Object],null)@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.c
ache.html:8164 $fireEvent_0([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:8249 $fireEvent([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:8046 $fireEvent_1([object Object],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:18414 fireEvent_3([object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:18600 fireNativeEvent([object MouseEvent],[object Object],[object 
HTMLDivElement])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3
A.cache.html:7003 $onBrowserEvent([object Object],[object 
MouseEvent])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.ca
che.html:18474 onBrowserEvent([object 
MouseEvent])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.ca
che.html:18630 dispatchEventImpl([object MouseEvent],[object 
HTMLDivElement],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:16638 dispatchEvent_0([object MouseEvent],[object HTMLDivElement],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:16629 ([object 
MouseEvent])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.ca
che.html:17720 apply((function (evt) {var listener, curElem = this, 
stackIndex;$stack_0[stackIndex = ++$stackDepth_1] = null;while 
(($location_0[stackIndex] = "DOMImplStandard.java:177", curElem) && !(listener 
= curElem.__listener)) {curElem = ($location_0[stackIndex] = 
"DOMImplStandard.java:178", curElem).parentNode;}if (($location_0[stackIndex] = 
"DOMImplStandard.java:175", curElem) && curElem.nodeType != 1) 
{$location_0[stackIndex] = "DOMImplStandard.java:182", curElem = null;}if 
($location_0[stackIndex] = "DOMImplStandard.java:175", listener) {if 
(isMyListener(($location_0[stackIndex] = "DOMImplStandard.java:185", 
listener))) {dispatchEvent_0(($location_0[stackIndex] = 
"DOMImplStandard.java:187", evt), curElem, listener);}}$stackDepth_1 = 
stackIndex - 1;}),[object HTMLDivElement],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:2989 entry0((function (evt) {var listener, curElem = this, 
stackIndex;$stack_0[stackIndex = ++$stackDepth_1] = null;while 
(($location_0[stackIndex] = "DOMImplStandard.java:177", curElem) && !(listener 
= curElem.__listener)) {curElem = ($location_0[stackIndex] = 
"DOMImplStandard.java:178", curElem).parentNode;}if (($location_0[stackIndex] = 
"DOMImplStandard.java:175", curElem) && curElem.nodeType != 1) 
{$location_0[stackIndex] = "DOMImplStandard.java:182", curElem = null;}if 
($location_0[stackIndex] = "DOMImplStandard.java:175", listener) {if 
(isMyListener(($location_0[stackIndex] = "DOMImplStandard.java:185", 
listener))) {dispatchEvent_0(($location_0[stackIndex] = 
"DOMImplStandard.java:187", evt), curElem, listener);}}$stackDepth_1 = 
stackIndex - 1;}),[object HTMLDivElement],[object 
Object])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.cache.
html:3045 ([object 
MouseEvent])@http://localhost:9898/webclient/368DF6B4111882D38F87DA72CB652E3A.ca
che.html:3026
Unknown.$collect (JsArrayString.java:42)
Unknown.fillInStackTrace_2 (StackTraceCreator.java:147)
Unknown.fillInStackTrace_0 (StackTraceCreator.java:387)
Unknown.fillInStackTrace (Throwable.java:72)
Unknown.$$init_16 (Throwable.java:46)
Unknown.Throwable_0 (Throwable.java:49)
Unknown.Exception_0 (Exception.java:25)
Unknown.RuntimeException_0 (RuntimeException.java:25)
Unknown.JavaScriptException_0 (JavaScriptException.java:127)
Unknown.caught_0 (Exceptions.java:29)
Unknown.$setBaseAndExtent (SelectionW3CNative.java:200)
Unknown.$setAnchorAndFocus (SelectionW3CNative.java:127)
Unknown.$set_8 (SelectionImplW3C.java:167)
Unknown.set_16 (SelectionImplW3C.java:130)
Unknown.set_13 (NativeSelectionUtil.java:220)
Unknown.setCaret_5 (NativeSelectionUtil.java:230)
Unknown.$setCaret (PassiveSelectionHelper.java:272)
Unknown.setCaret_0 (PassiveSelectionHelper.java:261)
Unknown.$safelyRestoreSelection (EditorImpl.java:1960)
Unknown.focus_4 (EditorImpl.java:1915)
Unknown.$startNewSession (EditSession.java:141)
Unknown.$startEditing_1 (EditSession.java:113)
Unknown.$focusAndEdit (Actions.java:150)
Unknown.$reply (Actions.java:99)
Unknown.onMouseDown_3 (MenuController.java:73)
Unknown.$dispatch_23 (EventDispatcherPanel.java:271)
Unknown.dispatch_22 (EventDispatcherPanel.java:270)
Unknown.$dispatch_20 (EventDispatcherPanel.java:175)
Unknown.onMouseDown (EventDispatcherPanel.java:276)
Unknown.$dispatch_6 (MouseDownEvent.java:54)
Unknown.dispatch_6 (MouseDownEvent.java:53)
Unknown.$doFire (SimpleEventBus.java:205)
Unknown.$fireEvent_0 (SimpleEventBus.java:103)
Unknown.$fireEvent (HandlerManager.java:101)
Unknown.$fireEvent_1 (Widget.java:103)
Unknown.fireEvent_3 (Widget.java:101)
Unknown.fireNativeEvent (DomEvent.java:116)
Unknown.$onBrowserEvent (Widget.java:151)
Unknown.onBrowserEvent (Widget.java:137)
Unknown.dispatchEventImpl (DOM.java:1308)
Unknown.dispatchEvent_0 (DOM.java:1264)
Unknown.anonymous (DOMImplStandard.java:187)
Unknown.apply (Impl.java:168)
Unknown.entry0 (Impl.java:214)
Unknown.anonymous (Impl.java:57)

Original comment by monami.s...@gmail.com on 17 Feb 2011 at 11:18

GoogleCodeExporter commented 9 years ago
That error occurs when loading the dev-built webclient in Firefox, because 
compile-gwt-dev only compiles a version for Webkit browsers.

Can you load the client successfully in Safari or Chrome?

If you want to develop in Firefox, you can either use the full GWT build 
(compile-gwt instead of compile-gwt-dev), which takes a long time to run, or 
you can edit box/webclient/WebClientDev.gwt.xml and change the value of 
"user.agent" from "safari" to "gecko1_8".

Original comment by hearn...@google.com on 17 Feb 2011 at 11:38

GoogleCodeExporter commented 9 years ago
Ok... So this time I tested both the windows in Chrome and this is what I found.

If I reply by Clicking on "Click here to reply" everything works fine !
But if I reply by clicking on "Reply" then I get :
Received deltas with no stacklet present!
  Unknown.$collect (JsArrayString.java:42)
  Unknown.fillInStackTrace_2 (StackTraceCreator.java:147)
  Unknown.fillInStackTrace_0 (StackTraceCreator.java:387)
  Unknown.fillInStackTrace (Throwable.java:72)
  Unknown.$$init_16 (Throwable.java:46)
  Unknown.Throwable_1 (Throwable.java:52)
  Unknown.Exception_1 (Exception.java:29)
  Unknown.RuntimeException_1 (RuntimeException.java:29)
  Unknown.IllegalStateException_1 (IllegalStateException.java:28)
  Unknown.onUpdate_4 (OperationChannelMultiplexerImpl.java:504)
  Unknown.onUpdate_5 (ViewChannelImpl.java:329)
  Unknown.onWaveletUpdate_0 (RemoteWaveViewService.java:304)
  Unknown.onWaveletUpdate (RemoteViewServiceMultiplexer.java:101)
  Unknown.onMessage_3 (WaveWebSocketClient.java:143)
  Unknown.onMessage_1 (WaveSocketFactory.java:65)
  Unknown.onMessage (GWTSocketIOConnectionImpl.java:120)
  Unknown.anonymous (GWTSocketIOConnectionImpl.java:17)
  Unknown.apply (Impl.java:168)
  Unknown.entry0 (Impl.java:214)
  Unknown.anonymous (Impl.java:57)

Original comment by monami.s...@gmail.com on 18 Feb 2011 at 12:19