1328899 - Clicking in a <textarea> flashes orange on Ubuntu

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P4)

Description

[Jonathan Kingston [:jkt] he/him]

STR:

1. Go to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/textarea
2. Scroll to <textarea> example
3. Click in the textarea

Expected results
Text highlight to be on the text only

Actual result
Flash of orange whilst mousedown which is Ubuntu's highlight text colour. Highlighting text works however only after seeing this flicker.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3ccb3bbe75fa1bbc248285900284646bfdaf56b4&tochange=5db66203614e7591bf7bfb2de3f802f144ae139c

Suspect: cd7544affe40	Martin Stransky — Bug 1288412 - Move GtkScrolledWindow to WidgetCache, r=acomminos

Comment 2

[Martin Stránský [:stransky] (ni? me)]

Which Ubuntu version do you run?

Comment 3

[Jonathan Kingston [:jkt] he/him]

I am using:
Ubuntu 16.04.

Couldn't replicate in:
51.0b11 Fedora
45.6.0 ESR Debian
Central build Debian

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

It is a bit late in the beta 51 cycle but we could still take a patch for 52/53.

Comment 5

[Karl Tomlinson (:karlt)]

I suspect this is due to the change in logic for determining GtkStateFlags.

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Attachment: patch

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8825344 [details] [diff] [review]
patch

>+    // Workaround for Bug 1328899 - We don't want "selected" background color
>+    // for active view class here which is set by Ambiance theme.

Usually "workaround" refers a something that avoids a bug in something else,
and usually that something else is a different product and so the fix cannot
be applied there.  This comment therefore implies a bug in the Ambiance theme,
but I don't think that would be a fair accusation.

The problem here is that Gecko is not generating the same state flags as what
GTK does for similar widgets.

>+    if (state->focused && !state->disabled) {
>+        state_flags = static_cast<GtkStateFlags>(GTK_STATE_FLAG_FOCUSED);
>+    }

This is a specific solution for just part of a more general problem in the way
that Gecko is determining the state flags.

Since cd7544affe40, Gecko is also including GTK_STATE_FLAG_PRELIGHT (even when not focused), but GTK does not include this, and so Gecko may pick up a general :hover or :prelight selector in a theme that the author never expected to be applied to a textview-like widget.

The same issues with _PRELIGHT and _ACTIVE also apply to other nodes in this
function at least.  The gtk_render_background() with the style for
MOZ_GTK_TEXT_VIEW is demonstrates this bug with Ambiance for GTK
3.20 in Ubuntu 10.10 because of these extra state flags.  Similarly gtk_render_frame() with the style for MOZ_GTK_SCROLLED_WINDOW should have neither _PRELIGHT nor _ACTIVE (because GTK does not set these).

Perhaps other widgets have similar issues, but I think a fix for this bug
should address all of these issues in moz_gtk_text_view_paint() because that
is what changed in cd7544affe40.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Okay. Unfortunately I won't have had time for that until next week so if we want the fix faster we'll need to take the workaround or somebody else has to step in.

Comment 9

[Karl Tomlinson (:karlt)]

No problem.  I don't think we're going to put this on 51 anyway, but I wrote a patch.  I'll flag you for review, when you get a chance.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc541016b61e4376288f496ec6a8465e7c427055

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: bug 1328899 don't use GTK prelight and active state flags when drawing textfield-multiline background

This is consistent with GTK not setting these flags with GtkTextView
and GtkScrolledWindow.

The active flag was triggering @selected_bg_color with Ambiance.

This patch almost reverts the state flags to the logic prior to
cd7544affe40 but keeps the focused flag in the scrolled window.
A parent scrolled window is not typically focused when a text view
has focus, but this may make the focus clearer with some themes
as text view backgrounds do not typically render differently with
focus.

Review commit: https://reviewboard.mozilla.org/r/104462/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104462/

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8826502 [details]
bug 1328899 don't use GTK prelight and active state flags when drawing textfield-multiline background

https://reviewboard.mozilla.org/r/104462/#review105678

Well, I really think this is an Ambiance theme bug. Gtk does not restrict which states can or can't be set on any widget and/or CSS node and the states comes from .view class which is also applied to GtkTextView background here. This bug is not shown on Adwaita for instance. I'm not sure Ubuntu is willing to fix the Ambiance theme so we may need to take this workaround for that plus we use that before.

So r+ but please add a comment why we use the state restriction here. If the original code was documented I wouldn't break it by 1288412.

Comment 12

[Karl Tomlinson (:karlt)]

Comment on attachment 8826502 [details]
bug 1328899 don't use GTK prelight and active state flags when drawing textfield-multiline background

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104462/diff/1-2/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af6601db2875
don't use GTK prelight and active state flags when drawing textfield-multiline background r=stransky+263117

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/af6601db2875

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Too late for 51 (RC2 is about to be built). Please request Aurora approval on this when you get a chance, though.

Comment 16

[Martin Stránský [:stransky] (ni? me)]

Sure, but I can't find the uplift form and google doesn't help me much. Shall I set any flag or can you please point me to the form we used? Thanks.

Comment 17

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8826502 [details]
bug 1328899 don't use GTK prelight and active state flags when drawing textfield-multiline background

Approval Request Comment
[Feature/Bug causing the regression]: Gtk3
[User impact if declined]: Visual distraction on text entry element on html forms in Ubuntu 16.04
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: Click by mouse at textarea element (data:text/html,<textarea%20name="textarea"%20rows="10"%20cols="50">Write%20something%20here</textarea>) on Ubuntu 16.04 with Ambiance theme.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: Not risky, we only remove extra style flags from textarea widget. We used to have similar fix here before.
[String changes made/needed]: None

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 8826502 [details]
bug 1328899 don't use GTK prelight and active state flags when drawing textfield-multiline background

gtk3 fix for aurora52

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ccec56819e76

Comment 21

[Andrei Vaida [:avaida]]

I've reproduced the issue on an affected build (50.1.0, 20161208153507) using the page from Comment 0 on Ubuntu 16.04 x64.

This is verified fixed on Ubuntu 16.04 x64 using:

    - 52.0b8-build1 (20170220070057),
    - 53.0a2 (2017-02-22)

and the textarea is no longer flashing orange on select.