Closed
Bug 574230
Opened 14 years ago
Closed 14 years ago
PopupNotifications should apply some attribute to its icon box
Categories
(Toolkit :: UI Widgets, defect)
Toolkit
UI Widgets
Tracking
()
VERIFIED
FIXED
mozilla2.0b1
People
(Reporter: mossop, Assigned: Gavin)
References
Details
(Whiteboard: [doorhanger])
Attachments
(2 files)
4.51 KB,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
2.02 KB,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
It strikes me that we may want many notifications all attached to the same place that the geo ones are attached. Currently anything using the same PopupNotifications gets a geo icon there. If PopupNotifications added an attribute to the box as it unhides it, say popupid, then we'd be able to pick and choose what icon to show there.
Assignee | ||
Comment 1•14 years ago
|
||
This was the intent, yes, I just didn't follow through completely.
Assignee | ||
Comment 2•14 years ago
|
||
Comment 3•14 years ago
|
||
Comment on attachment 453622 [details] [diff] [review] patch >+#notification-popup-box > * { Can we have a common class name for these nodes?
![]() |
||
Updated•14 years ago
|
Blocks: sm-doorhanger
![]() |
||
Comment 4•14 years ago
|
||
Comment on attachment 453622 [details] [diff] [review] patch >+#notification-popup-box > * { I agree with dao, can we please have a common class set on those? Also, any reason why those CSS rules are not in immediate vicinity to the existing #notification-popup-box rule? >+ // Test that anchor icon appears >+ { // Test #11 Why "Test #11" when the one immediately before it is "Test #9"?
![]() |
||
Comment 5•14 years ago
|
||
(In reply to comment #4) > Why "Test #11" when the one immediately before it is "Test #9"? Oops, I didn't seem to have bug 574228 locally yet, so ignore that one.
Assignee | ||
Comment 6•14 years ago
|
||
I'm not sure I understand why you want the class. Is it just for the selector efficiency? All items in the notification-popup-box should be treated the same, so the downside to using a class is that it requires remembering to add the class when making additions to the box (whereas if you forget about the styling with the current patch, you won't see the added icon at all). If the perf difference was significant I'd be all for it, but I don't think it is, so I don't really see any advantages to using a class name.
![]() |
||
Comment 7•14 years ago
|
||
IIRC, any * at the last place in the cascade (i.e. the first one that is checked by the style system, AFAIK) is always to be avoided as it's slowing down all style lookups in that scope.
Assignee | ||
Comment 8•14 years ago
|
||
Yes, I know the efficiency differences. My impression is that they're insignificant and thus outweighed by my maintainability argument in comment 6, but I don't feel strongly. I'll let Dao make the call.
Attachment #453954 -
Flags: review?(dao)
Updated•14 years ago
|
Attachment #453622 -
Flags: review?(dao) → review+
Comment 10•14 years ago
|
||
Comment on attachment 453954 [details] [diff] [review] with class I don't know whether the difference in efficiency is significant, but I'm leaning towards saving the CPU cycles either way. It seems that if you miss the class and the icon never hides, you'll notice this right away, and in most cases you'd probably just copy an existing icon and change the id anyway.
Attachment #453954 -
Flags: review?(dao) → review+
Assignee | ||
Comment 11•14 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ecbfb0070431
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a6
You need to log in
before you can comment on or make changes to this bug.
Description
•