Closed GoogleCodeExporter closed 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
Example blip: wave://waveinabox.net/w+2-W46qg1MnA/~/conv+root/b+1njtdj3ecxjbiA
Original comment by vega113
on 18 Apr 2011 at 8:32
I should have time to look at this issue tomorrow (4/19).
Original comment by michael.macfadden
on 18 Apr 2011 at 8:37
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
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
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
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
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
fixed by 618050651f4d
Original comment by vega113
on 21 Apr 2011 at 7:46
Original issue reported on code.google.com by
vega113
on 18 Apr 2011 at 8:29