Closed
Bug 1328899
Opened 8 years ago
Closed 8 years ago
Clicking in a <textarea> flashes orange on Ubuntu
Categories
(Core :: Widget: Gtk, defect, P4)
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: jkt, Assigned: stransky)
References
()
Details
(Keywords: regression, Whiteboard: tpi:+)
Attachments
(2 files)
1.49 KB,
patch
|
karlt
:
review-
|
Details | Diff | Splinter Review |
59 bytes,
text/x-review-board-request
|
stransky
:
review+
jcristau
:
approval-mozilla-aurora+
|
Details |
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•8 years ago
|
||
regression-window |
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
Blocks: 1288412
status-firefox50:
--- → wontfix
status-firefox51:
--- → affected
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox-esr45:
--- → unaffected
tracking-firefox51:
--- → ?
tracking-firefox52:
--- → ?
tracking-firefox53:
--- → ?
Flags: needinfo?(stransky)
Keywords: regression
Version: 52 Branch → 50 Branch
Updated•8 years ago
|
Assignee | ||
Comment 2•8 years ago
|
||
Which Ubuntu version do you run?
Flags: needinfo?(stransky) → needinfo?(jkt)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → stransky
Reporter | ||
Comment 3•8 years ago
|
||
I am using:
Ubuntu 16.04.
Couldn't replicate in:
51.0b11 Fedora
45.6.0 ESR Debian
Central build Debian
Flags: needinfo?(jkt)
Comment 4•8 years ago
|
||
It is a bit late in the beta 51 cycle but we could still take a patch for 52/53.
Comment 5•8 years ago
|
||
I suspect this is due to the change in logic for determining GtkStateFlags.
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8825344 -
Flags: review?(karlt)
Comment 7•8 years ago
|
||
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.
Attachment #8825344 -
Flags: review?(karlt) → review-
Assignee | ||
Comment 8•8 years ago
|
||
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.
Updated•8 years ago
|
Priority: -- → P4
Whiteboard: tpi:+
Comment 9•8 years ago
|
||
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 hidden (mozreview-request) |
Assignee | ||
Comment 11•8 years ago
|
||
mozreview-review |
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.
Attachment #8826502 -
Flags: review?(stransky) → review+
Comment hidden (mozreview-request) |
Comment 13•8 years ago
|
||
Pushed by ktomlinson@mozilla.com:
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•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 15•8 years ago
|
||
Too late for 51 (RC2 is about to be built). Please request Aurora approval on this when you get a chance, though.
Flags: needinfo?(stransky)
Assignee | ||
Comment 16•8 years ago
|
||
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.
Flags: needinfo?(stransky) → needinfo?(ryanvm)
Assignee | ||
Comment 17•8 years ago
|
||
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
Flags: needinfo?(ryanvm)
Attachment #8826502 -
Flags: approval-mozilla-aurora?
Comment 18•8 years ago
|
||
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
Attachment #8826502 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 19•8 years ago
|
||
bugherder uplift |
Comment 21•8 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•