Closed Bug 817584 Opened 7 years ago Closed 7 years ago

Replace descendent selectors with child selectors

Categories

(Firefox :: Downloads Panel, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: mconley, Assigned: mconley)

Details

Attachments

(1 file, 1 obsolete file)

We've got a few places in our downloads panel CSS where we use descendent selectors instead of child selectors, and according to https://developer.mozilla.org/en/Writing_Efficient_CSS#Avoid_the_descendant_selector, this is something we'd like to avoid.
Attached patch Patch v1 (obsolete) — Splinter Review
Comment on attachment 687778 [details] [diff] [review]
Patch v1

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

So these are the easiest ones to remove. There are a few more descendent selectors sprinkled around in these downloads.css files (example: http://mxr.mozilla.org/comm-central/source/mozilla/browser/themes/gnomestripe/downloads/downloads.css#176), but they've been there for quite a while. Is it worth replacing those as well?
Attachment #687778 - Flags: review?(mak77)
Comment on attachment 687778 [details] [diff] [review]
Patch v1

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

Let's replace whatever we found can be optimized, this is going to happen soon or later. Its unfortunate the id selector can't be optimized :(
Attachment #687778 - Flags: review?(mak77) → review+
Attached patch Patch v2Splinter Review
So I found a few more places where we could optimize by replacing descendent selectors with child selectors. This looks OK on gnomestripe. Going to make sure this doesn't break winstripe and pinstripe, and then I'll r? again.
Attachment #687778 - Attachment is obsolete: true
Checks out on pinstripe. Winstripe is next.
Comment on attachment 687815 [details] [diff] [review]
Patch v2

Alright, this seems to work alright on Windows 7 and XP, aero and classic. Ready for a(nother) review.
Attachment #687815 - Flags: review?(mak77)
Comment on attachment 687815 [details] [diff] [review]
Patch v2

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

I think you forgot themes/winstripe/downloads/downloads-aero.css?
(In reply to Marco Bonardo [:mak] from comment #7)
> Comment on attachment 687815 [details] [diff] [review]
> Patch v2
> 
> Review of attachment 687815 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think you forgot themes/winstripe/downloads/downloads-aero.css?

So, part of the fix for downloads-aero already landed here as part of the follow-up for bug 815131:

https://hg.mozilla.org/integration/mozilla-inbound/rev/69de9132d9e0

Did you mean these two large rules?:

https://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/downloads/downloads-aero.css#29
Comment on attachment 687815 [details] [diff] [review]
Patch v2

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

Ah ok, sorry, mxr was not yet up-to-date with the downloads-aero.css change
Attachment #687815 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/214cf10e5d17
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
You need to log in before you can comment on or make changes to this bug.