Last Comment Bug 710041 - Build fixes for gonk widget backend
: Build fixes for gonk widget backend
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla11
Assigned To: Michael Wu [:mwu]
:
Mentors:
Depends on:
Blocks: 709468 694206
  Show dependency treegraph
 
Reported: 2011-12-12 16:07 PST by Philipp von Weitershausen [:philikon]
Modified: 2011-12-16 05:56 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (4.59 KB, patch)
2011-12-12 16:28 PST, Philipp von Weitershausen [:philikon]
cjones.bugs: review+
Details | Diff | Review
v2 (1.78 KB, patch)
2011-12-15 15:38 PST, Michael Wu [:mwu]
no flags Details | Diff | Review

Description Philipp von Weitershausen [:philikon] 2011-12-12 16:07:30 PST

    
Comment 1 Philipp von Weitershausen [:philikon] 2011-12-12 16:28:28 PST
Created attachment 581088 [details] [diff] [review]
v1

This is one of the remaining pieces that was landed in cjones's m-c fork but hasn't made it to mozilla-central proper yet. For any review questions, please ask mwu... I'm just filing the patch for him :)
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-14 07:21:02 PST
Comment on attachment 581088 [details] [diff] [review]
v1

Huh ... I sure thought I had posted my review here already ...

>diff --git a/widget/src/gonk/nsWindow.h b/widget/src/gonk/nsWindow.h

>+extern nsIntRect gScreenBounds;
>+

This is so ugly.  Please file a followup to get rid of it.

>diff --git a/widget/src/xpwidgets/nsBaseWidget.h b/widget/src/xpwidgets/nsBaseWidget.h

>+  NS_IMETHOD_(void) SetInputContext(const InputContext& aContext,
>+                                    const InputContextAction& aAction) { return; }
>+  NS_IMETHOD_(InputContext) GetInputContext()
>+  {
>+    InputContext ctx;
>+    return ctx;
>+  }

As far as I can tell, this change is unnecessary.  Please remove it.

r=me with that fix.
Comment 3 Michael Wu [:mwu] 2011-12-14 13:59:19 PST
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> Comment on attachment 581088 [details] [diff] [review]
> v1
> 
> Huh ... I sure thought I had posted my review here already ...
> 
Looks like you posted it to another bug https://bugzilla.mozilla.org/show_bug.cgi?id=709585#c13

> >diff --git a/widget/src/gonk/nsWindow.h b/widget/src/gonk/nsWindow.h
> 
> >+extern nsIntRect gScreenBounds;
> >+
> 
> This is so ugly.  Please file a followup to get rid of it.
> 
Hmm, not sure why this is even in this patch. Slightly confused. I'll file a followup if this is necessary.

> >diff --git a/widget/src/xpwidgets/nsBaseWidget.h b/widget/src/xpwidgets/nsBaseWidget.h
> 
> >+  NS_IMETHOD_(void) SetInputContext(const InputContext& aContext,
> >+                                    const InputContextAction& aAction) { return; }
> >+  NS_IMETHOD_(InputContext) GetInputContext()
> >+  {
> >+    InputContext ctx;
> >+    return ctx;
> >+  }
> 
> As far as I can tell, this change is unnecessary.  Please remove it.
> 

The problem is that Gonk doesn't implement it because it doesn't need to communicate with a "native" IME, and the code won't build without a stub somewhere. I can just move this stub to Gonk though, since this would probably be the only platform without need to interface with external IMEs.
Comment 4 Philipp von Weitershausen [:philikon] 2011-12-14 14:05:50 PST
(In reply to Michael Wu [:mwu] from comment #3)
> > >diff --git a/widget/src/gonk/nsWindow.h b/widget/src/gonk/nsWindow.h
> > 
> > >+extern nsIntRect gScreenBounds;
> > >+
> > 
> > This is so ugly.  Please file a followup to get rid of it.
> > 
> Hmm, not sure why this is even in this patch.

Because I made the patch and didn't really know which bits belonged where. Thanks for sorting it out, mwu!
Comment 5 Michael Wu [:mwu] 2011-12-15 15:17:54 PST
(In reply to Michael Wu [:mwu] from comment #3)
> > >diff --git a/widget/src/xpwidgets/nsBaseWidget.h b/widget/src/xpwidgets/nsBaseWidget.h
> > 
> > >+  NS_IMETHOD_(void) SetInputContext(const InputContext& aContext,
> > >+                                    const InputContextAction& aAction) { return; }
> > >+  NS_IMETHOD_(InputContext) GetInputContext()
> > >+  {
> > >+    InputContext ctx;
> > >+    return ctx;
> > >+  }
> > 
> > As far as I can tell, this change is unnecessary.  Please remove it.
> > 
> 
> The problem is that Gonk doesn't implement it because it doesn't need to
> communicate with a "native" IME, and the code won't build without a stub
> somewhere. I can just move this stub to Gonk though, since this would
> probably be the only platform without need to interface with external IMEs.

Oh, looks like I already have a stub in Gonk. I'll keep the gonk side stub.
Comment 6 Michael Wu [:mwu] 2011-12-15 15:38:09 PST
Created attachment 582129 [details] [diff] [review]
v2

Review comments addressed.

Note You need to log in before you can comment on or make changes to this bug.