ui/gtk3: Refactor color of a selected candidate on lookup window

ui/gtk3: Refactor color of a selected candidate on lookup window

Recenly 'background-color' CSS property does not indicate the actual
background color in GTK 3.22 and IBus uses '-gtk-secondary-caret-color'
CSS property instead.
But I found every themes do not have '-gtk-secondary-caret-color'
CSS property even if the latest GTK is used. Some themes, e.g.
distro themes or third party themes,  uses 'background-color' CSS
property for the background color of the selected text.

This patch compares the colors of the normal 'background-color' and
the selected 'background-color' and in case they are different,
uses the selected 'background-color', otherwise the selected
'-gtk-secondary-caret-color'.
Also now IBus checks the colors of a dummy GtkTextView rather than
ones of GtkLabel on the lookup window because some themes
do not configure the selected bacground color in GtkLabel.

BUG=https://github.com/ibus/ibus/issues/1871

Committed: bdc83bcbe3c9f07a267499c297db12c68af82fa2

[Peng, 2017-01-18 15:24:55]

https://codereview.appspot.com/314280043/diff/1/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/1/ui/gtk3/candidatearea.vala#ne...
ui/gtk3/candidatearea.vala:126: context.get(Gtk.StateFlags.SELECTED, "color",
out color);
Is it possible to construct the StyleContext directly instead of using a dummy
TextView (If it is not too difficult)? Maybe you could check gtk code to find
out how TextView construct the StyleContext.

[fujiwara, 2017-01-23 12:13:16]

https://codereview.appspot.com/314280043/diff/1/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/1/ui/gtk3/candidatearea.vala#ne...
ui/gtk3/candidatearea.vala:126: context.get(Gtk.StateFlags.SELECTED, "color",
out color);
On 2017/01/18 15:24:55, Peng wrote:
> Is it possible to construct the StyleContext directly instead of using a dummy
> TextView (If it is not too difficult)? Maybe you could check gtk code to find
> out how TextView construct the StyleContext.

Done.

[Peng, 2017-01-25 00:31:16]

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.val...
ui/gtk3/candidatearea.vala:124: Gtk.StyleContext context = new
Gtk.StyleContext();
How about only create StyleContext and get the color from it once? In this
method, we could just use the stored color directly.
And you could connect GtkStyleContext::changed singal for updating those stored
colors.

[fujiwara, 2017-01-25 03:21:23]

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.val...
ui/gtk3/candidatearea.vala:124: Gtk.StyleContext context = new
Gtk.StyleContext();
On 2017/01/25 00:31:16, Peng wrote:
> How about only create StyleContext and get the color from it once? In this
> method, we could just use the stored color directly.
> And you could connect GtkStyleContext::changed singal for updating those
stored
> colors.

I'm not sure what you mean here.
The color is different between widgets and also inherited from the parent
widgets.
That's why I assigned Gtk.TextView to the StyleContext and the parent Gtk.Box to
its parent.
I think either new Gtk.StyleContext() or the context of Gtk.Label does not
return the right color.

[Peng, 2017-01-25 04:40:28]

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.val...
ui/gtk3/candidatearea.vala:124: Gtk.StyleContext context = new
Gtk.StyleContext();
On 2017/01/25 03:21:23, fujiwara wrote:
> On 2017/01/25 00:31:16, Peng wrote:
> > How about only create StyleContext and get the color from it once? In this
> > method, we could just use the stored color directly.
> > And you could connect GtkStyleContext::changed singal for updating those
> stored
> > colors.
> 
> I'm not sure what you mean here.
> The color is different between widgets and also inherited from the parent
> widgets.
> That's why I assigned Gtk.TextView to the StyleContext and the parent Gtk.Box
to
> its parent.
> I think either new Gtk.StyleContext() or the context of Gtk.Label does not
> return the right color.

I think the frequency of calling set_candidates() is very high. It is better to
avoid creating the StyleContext and getting the color every time. So my
suggestion is we could only create Gtk.StyleContext and get the color once and
save it, probably in recreate_ui() method. When set_candidates() is called, we
could just use the saved color. And the saved color could be updated in the
handler of Gtk.StyleContext::changed signal.

WDYT?

[fujiwara, 2017-01-25 12:20:00]

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/40001/ui/gtk3/candidatearea.val...
ui/gtk3/candidatearea.vala:124: Gtk.StyleContext context = new
Gtk.StyleContext();
On 2017/01/25 04:40:27, Peng wrote:
> On 2017/01/25 03:21:23, fujiwara wrote:
> > On 2017/01/25 00:31:16, Peng wrote:
> > > How about only create StyleContext and get the color from it once? In this
> > > method, we could just use the stored color directly.
> > > And you could connect GtkStyleContext::changed singal for updating those
> > stored
> > > colors.
> > 
> > I'm not sure what you mean here.
> > The color is different between widgets and also inherited from the parent
> > widgets.
> > That's why I assigned Gtk.TextView to the StyleContext and the parent
Gtk.Box
> to
> > its parent.
> > I think either new Gtk.StyleContext() or the context of Gtk.Label does not
> > return the right color.
> 
> I think the frequency of calling set_candidates() is very high. It is better
to
> avoid creating the StyleContext and getting the color every time. So my
> suggestion is we could only create Gtk.StyleContext and get the color once and
> save it, probably in recreate_ui() method. When set_candidates() is called, we
> could just use the saved color. And the saved color could be updated in the
> handler of Gtk.StyleContext::changed signal.
> 
> WDYT?

OK, I got it and updated.

[Peng, 2017-01-25 14:03:45]

LGMT with two comments.

https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.va...
ui/gtk3/candidatearea.vala:73: m_style_context.set_parent(null);
I thought the m_style_context will be destroy soon. Why we need set thet parent
to null? Could you please add a comment.

https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.va...
ui/gtk3/candidatearea.vala:348: m_style_context = new Gtk.StyleContext();
Sorry. I just realized we can move those code into the CandidateAura's
constructor. And the recreate_ui could be called more than once.

[Peng, 2017-01-25 14:04:02]

On 2017/01/25 14:03:45, Peng wrote:
> LGMT with two comments.
> 
>
https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.vala
> File ui/gtk3/candidatearea.vala (right):
> 
>
https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.va...
> ui/gtk3/candidatearea.vala:73: m_style_context.set_parent(null);
> I thought the m_style_context will be destroy soon. Why we need set thet
parent
> to null? Could you please add a comment.
> 
>
https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.va...
> ui/gtk3/candidatearea.vala:348: m_style_context = new Gtk.StyleContext();
> Sorry. I just realized we can move those code into the CandidateAura's
> constructor. And the recreate_ui could be called more than once.

Sorry. LGMY => LGTM

[fujiwara, 2017-01-27 04:29:34]

https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.vala
File ui/gtk3/candidatearea.vala (right):

https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.va...
ui/gtk3/candidatearea.vala:73: m_style_context.set_parent(null);
On 2017/01/25 14:03:44, Peng wrote:
> I thought the m_style_context will be destroy soon. Why we need set thet
parent
> to null? Could you please add a comment.

I added set_parent(null) because the behavior is not documented but I'm also
fine to delete this line.

https://codereview.appspot.com/314280043/diff/100001/ui/gtk3/candidatearea.va...
ui/gtk3/candidatearea.vala:348: m_style_context = new Gtk.StyleContext();
On 2017/01/25 14:03:44, Peng wrote:
> Sorry. I just realized we can move those code into the CandidateAura's
> constructor. And the recreate_ui could be called more than once.
> 

Done.

[fujiwara, 2017-01-27 05:43:07]

Committed patchset #7 (id:120001) manually as
bdc83bcbe3c9f07a267499c297db12c68af82fa2 (presubmit successful).