949462 - Twisties on scopes are invisible on Linux

Status: RESOLVED FIXED

(DevTools :: Object Inspector, defect)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Screenshot

I'm not sure whether this is a result of the recent theme-ification of the variables view, or if it used to look like that, but it is very annoying. You can see in the attached screenshot that only selecting a scope makes the arrow visible. Both themes are affected, as the scope background seems to be the same.

Comment 1

[Victor Porof [:vporof][:vp]]

It used to looks like that, and it definitely is pretty annoying.

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Victor Porof [:vp] from comment #1)
> It used to looks like that, and it definitely is pretty annoying.

Looks like there is an element with the 'arrow' class, which defines -moz-appearance: treetwisty - https://mxr.mozilla.org/mozilla-central/source/browser/themes/linux/devtools/widgets.css#642.  I'm not really sure why this doesn't show up on Linux, but can you maybe try just setting a color on the element and see if it starts showing up?

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Brian Grinstead [:bgrins] from comment #2)

It *does* show up, but the color is so similar to the scope's header background that it's practically invisible.

Comment 4

[Tim Nguyen :ntim]

Attachment: Patch v1

This fixes the issue, and removes some useless Windows code.

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8436306 [details] [diff] [review]
Patch v1

Review of attachment 8436306 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good on osx with the patch applied.  I would like you to refactor this a bit, using the existing .theme-twisty class provided by the light and dark themes.

Panos, does this fix the issues you were seeing on Linux?

::: browser/themes/shared/devtools/widgets.inc.css
@@ +893,5 @@
>  }
>  
>  /* Expand/collapse arrow */
>  
>  .arrow {

Could we just apply the 'theme-twisty' class to this object from js (in VariablesView.jsm) and only add the special margins needed using this selector: ".variables-view-container .theme-twisty"?

This would help clean up the code while still fixing the issue

Comment 6

[Tim Nguyen :ntim]

Attachment: Patch v2

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8437183 [details] [diff] [review]
Patch v2

Review of attachment 8437183 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this looks fine now. Thanks!

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8437183 [details] [diff] [review]
Patch v2

Review of attachment 8437183 [details] [diff] [review]:
-----------------------------------------------------------------

Code changes look good to me

Comment 9

[Tim Nguyen :ntim]

Try push : https://tbpl.mozilla.org/?tree=Try&rev=c7e2fd3b03a5

Comment 10

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #9)
> Try push : https://tbpl.mozilla.org/?tree=Try&rev=c7e2fd3b03a5

The DevTools tests seem to be failing, do you know what might have happened here ?

Comment 11

[Tim Nguyen :ntim]

Oh wait, I just figured it out myself. The tests check for the .arrow class.

Comment 12

[Tim Nguyen :ntim]

Attachment: Patch v3 (r=bgrins, ui-r=past)

Fixed failing tests.

Comment 13

[Tim Nguyen :ntim]

Try : https://tbpl.mozilla.org/?tree=Try&rev=4fcb53d5f031

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/fx-team/rev/64537f5eb321

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/64537f5eb321