Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Build fixes for gonk widget backend

RESOLVED FIXED in mozilla11

Status

()

Core
Widget
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: philikon, Assigned: mwu)

Tracking

(Blocks: 1 bug)

Trunk
mozilla11
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
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 :)
Attachment #581088 - Flags: review?(jones.chris.g)
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.
Attachment #581088 - Flags: review?(jones.chris.g) → review+
(Assignee)

Comment 3

6 years ago
(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.
(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!
(Assignee)

Comment 5

6 years ago
(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.
(Assignee)

Comment 6

6 years ago
Created attachment 582129 [details] [diff] [review]
v2

Review comments addressed.
Attachment #581088 - Attachment is obsolete: true
(Assignee)

Comment 7

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/10d8aa274341
https://hg.mozilla.org/mozilla-central/rev/10d8aa274341
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.