Closed Bug 949462 Opened 10 years ago Closed 10 years ago

Twisties on scopes are invisible on Linux

Categories

(DevTools :: Object Inspector, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 33

People

(Reporter: past, Assigned: ntim)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached image 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.
It used to looks like that, and it definitely is pretty annoying.
(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?
(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.
Attached patch Patch v1 (obsolete) — Splinter Review
This fixes the issue, and removes some useless Windows code.
Attachment #8436306 - Flags: review?(bgrinstead)
Assignee: nobody → ntim007
Status: NEW → ASSIGNED
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
Attachment #8436306 - Flags: review?(bgrinstead) → ui-review?(past)
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #8436306 - Attachment is obsolete: true
Attachment #8436306 - Flags: ui-review?(past)
Attachment #8437183 - Flags: ui-review?(past)
Attachment #8437183 - Flags: review?(bgrinstead)
Comment on attachment 8437183 [details] [diff] [review]
Patch v2

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

Yes, this looks fine now. Thanks!
Attachment #8437183 - Flags: ui-review?(past) → ui-review+
Comment on attachment 8437183 [details] [diff] [review]
Patch v2

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

Code changes look good to me
Attachment #8437183 - Flags: review?(bgrinstead) → review+
(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 ?
Flags: needinfo?(bgrinstead)
Oh wait, I just figured it out myself. The tests check for the .arrow class.
Flags: needinfo?(bgrinstead)
Fixed failing tests.
Attachment #8437183 - Attachment is obsolete: true
Attachment #8437846 - Flags: ui-review+
Attachment #8437846 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/64537f5eb321
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: