Open
Bug 877697
Opened 11 years ago
Updated 2 years ago
panelUIOverlay.inc.css uses far too many descendant selectors
Categories
(Firefox :: Theme, defect)
Tracking
()
NEW
People
(Reporter: mconley, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [Australis:P5])
I'm concerned about the high number of descendant selectors we have in browser/themes/shared/panelUIOverlay.inc.css.
https://developer.mozilla.org/en/Writing_Efficient_CSS tells us that descendant selectors are very expensive, and we should try writing something more specific.
Comment 1•11 years ago
|
||
Alternatively, could we use a scoped stylesheet here? That should make the impact of descendent selectors negligible.
Comment 2•11 years ago
|
||
Not taking for Australis:M7.
Updated•11 years ago
|
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P5]
Updated•11 years ago
|
Blocks: fxdesktopbacklog
Whiteboard: [Australis:M?][Australis:P5] → [Australis:M?][Australis:P5] [feature] p=0
Updated•11 years ago
|
No longer blocks: fxdesktopbacklog
Whiteboard: [Australis:M?][Australis:P5] [feature] p=0 → [Australis:M?][Australis:P5]
Updated•11 years ago
|
Whiteboard: [Australis:M?][Australis:P5] → [Australis:P5]
Updated•10 years ago
|
Mentor: manishearth
Whiteboard: [Australis:P5] → [Australis:P5][good first bug]
Updated•10 years ago
|
Whiteboard: [Australis:P5][good first bug] → [Australis:P5][good first bug][lang=css]
Comment 3•10 years ago
|
||
Manish I'm new and trying to get involved. Can I start working on this bug?
Comment 4•10 years ago
|
||
hi! I'm new here , Please assign this bug to me!
Comment 5•10 years ago
|
||
(In reply to Abhirav Kariya from comment #4)
> hi! I'm new here , Please assign this bug to me!
I think the new policy is to start work on this bug before we mark it as assigned. Also, Sean might be working on it (sorry Sean, I missed your comment). If so, you might want to pick a new bug.
Anyway, if you start working on this and put up a simple patch, I can mark it as assigned. Thanks for your interest!
Let me know if you need help.
Comment 6•10 years ago
|
||
I wanted to work on it, yes. I was waiting for a reply before I jumped in and worked on it. I didn't want to step on toes.
Thanks
Comment 7•10 years ago
|
||
(In reply to Sean Zaferopolos from comment #6)
> I wanted to work on it, yes. I was waiting for a reply before I jumped in
> and worked on it. I didn't want to step on toes.
Sorry about that, sure, start working on it :)
Abhrav, you might want to work on this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=908954
Comment 8•10 years ago
|
||
panelUIOverlay.inc.css. location? the location in description is wrong.
Comment 9•10 years ago
|
||
Comment 10•10 years ago
|
||
help me patch this bug as im new to all this!!
Comment 11•10 years ago
|
||
(In reply to Abhirav Kariya from comment #10)
> help me patch this bug as im new to all this!!
Currently Sean is working on this, maybe pick a different bug? Bugsahoy might help: www.joshmatthews.net/bugsahoy/
Comment 12•10 years ago
|
||
ok
Comment 13•10 years ago
|
||
hello Manish
I am new at this.
I would like to work on this bug.
Comment 14•10 years ago
|
||
Manish,
This has been on my to-do for a while and keeps getting pushed back. Someone else can do it. I will try again some other time.
Updated•10 years ago
|
Assignee: nobody → gopalmeena94
Comment 15•10 years ago
|
||
I have looked through the file.
I don't know where we have to change.
I need some examples.
Comment 16•10 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css
https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Writing_efficient_CSS#Avoid_the_descendant_selector.21
Firstly, notice that we have a lot of descendent selectors to .panelUI-grid. This is for all the toolbar buttons in the dropdown panel. Perhaps moving those selectors to a scoped stylesheet there (<style scoped>) would be better?
Also, we have cases like this: http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css#376 where there are a lot of descendent selectors using ids.
Enabling the Browser Toolbox in your build and poking around should help, since you can tweak the browser-level CSS.
Comment 17•10 years ago
|
||
In cases like these[1] it might make sense to give the .toolbar-icon in question its own class, and just use `.panelUI-grid .badge-toolbaricon`.
Most of the descendent selectors here can be fixed by splitting the file into three; one for the palette, one for the panel, and one for everything else, and putting the palette/panel ones as scoped stylesheets in their respective locations.
[1]: http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css#430
Comment 18•10 years ago
|
||
@Gopal do you still have any questions? Let me know if that's the case. If you need help you can ask me or :jaws in IRC. Thanks!
Comment 19•10 years ago
|
||
@ Manish , sir i have exams now. i will do it after 6-7 days
Comment 22•9 years ago
|
||
Hey, I would like to work on this bug......
Comment 23•9 years ago
|
||
(In reply to Akilan Elango [:falcon] from comment #22)
> Hey, I would like to work on this bug......
Awesome! Do you have your environment set up? If not, see https://developer.mozilla.org/en/docs/Simple_Firefox_build
The file you need to update is http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css, according to the instructions in comment 0 and comment 16. Please let me know if you have any further questions!
Flags: needinfo?(gopalmeena94)
Updated•9 years ago
|
Assignee: nobody → nanonikhil
Comment 24•9 years ago
|
||
Can you please post the links with updated line numbers because I think that the line numbers you were referring to previously have changed.
Also, in which files should scoped stylesheet be included? Can you include an example?
Flags: needinfo?(manishearth)
Flags: needinfo?(jaws)
Comment 25•9 years ago
|
||
Also, what's the file http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css actually for?
Comment 26•9 years ago
|
||
Stuff like http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css#412 . It's all over the file. `>` is a descendent selector.
The file handles the code for the panel shown by the hamburger dropdown, IIRC.
Flags: needinfo?(manishearth)
Updated•9 years ago
|
Flags: needinfo?(jaws) → needinfo?
Comment 27•9 years ago
|
||
(In reply to Manish Goregaokar [:manishearth] from comment #26)
> Stuff like
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/
> customizableui/panelUIOverlay.inc.css#412 . It's all over the file. `>` is a
> descendent selector.
Whoa, slow down. No, ">" is not a descendant selector.
"#foo #bar"
is using a descendant selector ("match if the element matches #bar, and any of its ancestors matches #foo")
"#foo > #bar"
is using a child selector ("match if the element matches #bar, and its parent matches #foo")
Flags: needinfo?
Comment 28•9 years ago
|
||
Oh, sorry, got confused there for a bit :s Thanks for the save.
Comment 29•9 years ago
|
||
Sorry but I'm still a bit confused. Can you please provide a small patch to demonstrate a change so that I can follow it and proceed accordingly? Thanks.
Comment 30•9 years ago
|
||
(In reply to Nikhil Gupta from comment #29)
> Sorry but I'm still a bit confused. Can you please provide a small patch to
> demonstrate a change so that I can follow it and proceed accordingly? Thanks.
Looking at this file again, I think this shouldn't be a good first bug. Different instances of problematic selectors will require different solutions here, and that will take some knowledge of CSS and the ability to look at the selectors on a case-by-case basis, by yourself. It isn't possible for me to tell you "fix this selector this way" and then for you to fix all the other selectors the same way.
Would you consider working on bug 1178023 instead? There is already a suggestion there for what needs to change, you would just need to create an actual patch there and flag me for review. I'll needinfo you on that bug instead of here.
Assignee: nanonikhil → nobody
Whiteboard: [Australis:P5][good first bug][lang=css] → [Australis:P5][lang=css]
Comment 31•9 years ago
|
||
Hi, I am new to fixing bugs and I would like to work on the bug please.
Comment 32•9 years ago
|
||
After some discussion, this is probably not as easy as we thought it was initially.
Sorry about that.
Mentor: manishearth
Whiteboard: [Australis:P5][lang=css] → [Australis:P5]
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•