bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

Cleanup autocomplete css

RESOLVED FIXED in Firefox 56



Developer Tools: Shared Components
11 months ago
9 months ago


(Reporter: ntim, Assigned: Ruturaj Vartak)


Firefox 56

Firefox Tracking Flags

(firefox56 fixed)



(1 attachment)



11 months ago
See bug 1354504 comment 41. We may want to remove code that adds the theme classes as well.

Comment 1

9 months ago
Hey Tim,
Referring to bug 1354504 comment 38, The CSS isn't present perhaps got changed in some other commit.
Can I know what the fix for this bug now changes to?
Flags: needinfo?(ntim.bugs)

Comment 2

9 months ago
(In reply to Ruturaj Vartak from comment #1)
> Hey Tim,
> Referring to bug 1354504 comment 38, The CSS isn't present perhaps got
> changed in some other commit.
FYI, the last commit which changed the CSS significantly was bug 1354504: https://hg.mozilla.org/mozilla-central/filelog/58c5151bfd62de934b2286dbd664e69886270e28/devtools/client/themes/common.css

> Can I know what the fix for this bug now changes to?

- Removing the code that adds the theme classes to the popup
- Simplifying box-shadow and cursor rules

You might want to ask Julian about this.
Flags: needinfo?(ntim.bugs)

Comment 3

9 months ago
Created attachment 8878769 [details] [diff] [review]

Hey Julian from your comments below

> 1 - We already have a box-shadow defined for .devtools-autocomplete-popup on line 45 (with a slightly different value 0 1px 0 hsla(209,29%,72%,.25) inset;)

> 2 - I think this particular box-shadow here was only set for .devtools-autocomplete-popup in light-theme. Is there a good reason to also apply it to CodeMirror-hints & CodeMirror-Tern-tooltip? Both these classes already have box-shadows defined in light-theme.css and dark-theme.css

> 3 - In case you need to target .devtools-autocomplete-popup in the light-theme only, you should know that the light-theme classname is also added to the container when we are using the Firebug theme (an easy way for the firebug theme to inherit from the light theme). That means that you either need to have a very specific .light-theme:not(.firebug-theme) in your selector, or to override the value for firebug-theme.

> Now, I'm being very nit-picky here, for the sake of explaining the impact these kind of changes can have. We're talking about an almost invisible inset box shadow here, I don't think anybody would notice the difference between before and after your change :) So I don't really ask you to keep the colors 100% as they were before, let's just make sure we don't redefine the box-shadow property several times for no reason. To be honest I would just get rid of this box shadow: it's almost not noticeable, and when it is it feels dated. But it is easier if we do this kind of changes in a separate changeset.

It seems removal of box-shadow is the only thing. Could you please have a look?

Assignee: nobody → ruturaj
Attachment #8878769 - Flags: review?(jdescottes)
Comment on attachment 8878769 [details] [diff] [review]

Review of attachment 8878769 [details] [diff] [review]:

Thanks for the patch! 
Yes looks like this is the only thing left to update :)
Attachment #8878769 - Flags: review?(jdescottes) → review+


9 months ago
Keywords: checkin-needed

Comment 5

9 months ago
Thanks Julian

Comment 6

9 months ago
Pushed by ryanvm@gmail.com:
Cleanup autocomplete css. r=jdescottes
Keywords: checkin-needed

Comment 7

9 months ago
Last Resolved: 9 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.