Closed
Bug 852420
Opened 8 years ago
Closed 8 years ago
Suppress NeededToWrapXUL warning for generated content
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: MattN, Assigned: Gijs)
References
()
Details
(Whiteboard: [Australis:M3])
Attachments
(2 files)
3.06 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
2.12 KB,
patch
|
Details | Diff | Splinter Review |
While working on the new Firefox tab shape, there was a regular spew of the following warning: "XUL box for _moz_generated_content_after element contained an inline #text child, forcing all its children to be wrapped in a block." After talking with Boris, my understanding is that these can be suppressed for generated content. (see below) #developers 2013-02-01: [12:55:36] <mconley> bz: hey - so, I'm getting this warning in the error console: "Warning: XUL box for _moz_generated_content_after element contained an inline #text child, forcing all its children to be wrapped in a block." using ::after with content: ''…how does one avoid this? [12:56:49] <bz> mconley: You don't? [12:57:01] <bz> mconley: I assume you're using display:-moz-box on the :after or something? [12:57:09] <mconley> that's correct. [12:57:21] <bz> mconley: The warning is just saying there's a potential perf issue… [12:57:29] <bz> mconley: I suppose we could suppress them for generated content… [13:04:32] <bz> What happens if you set the ::after to be an inline-block? [13:04:42] <bz> Does it just not stretch vertically to the right height? [13:05:50] <mconley> bz: yes, it's a XUL hbox. Setting it as inline-block gets rid of the warning, but causes some stuff to shift around. Ok, we're going to investigate that route a little deeper though. Thanks! After the discussion we went with inline-block but Dão suggested in bug 738491 comment 145 that mixing inline-block with -moz-box is not wise. You can take a look at the pseudo-elements using inline-block at https://bugzilla.mozilla.org/attachment.cgi?id=722702&action=diff#a/browser/themes/shared/tabs.inc.css_sec2 Could we get these warnings suppressed if they aren't relevant? Thanks.
![]() |
||
Updated•8 years ago
|
Flags: needinfo?(dbaron)
![]() |
||
Comment 1•8 years ago
|
||
Oh, so the reason I asked dbaron for feedback is that the warnings _are_ relevant: using an inline here is slower and uses more memory than using an inline-block. The question is whether for generated content in particular it's worth making an exception to the warning for some reason.
Reporter | ||
Updated•8 years ago
|
Whiteboard: [Australis:M3]
Comment 2•8 years ago
|
||
I guess I'm ok suppressing it, though it's sort of "incorrect XUL" based on the original design of XUL as I understand it. (Who owns XUL these days?)
Flags: needinfo?(dbaron)
Assignee | ||
Comment 3•8 years ago
|
||
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #741258 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•8 years ago
|
||
![]() |
||
Comment 5•8 years ago
|
||
Comment on attachment 741258 [details] [diff] [review] Patch r=me
Attachment #741258 -
Flags: review?(bzbarsky) → review+
Reporter | ||
Comment 6•8 years ago
|
||
Perhaps we could experiment with border-image for the separators instead now that we've switched to using an image for that since filing this bug. (Gradients are not supported in border-image in Gecko yet [bug 709587]). That is easier to maintain compared to the pseudo-elements but may cause new problems with tab overflow. I don't want to hide a warning that is relevant but I also don't really understand what exactly we're doing that is incorrect XUL. If it is just the |content| property that is causing an inline text child, then that could also change to url("chrome://browser/skin/tabbrowser/tab-separator.png") now. Gijs, can you take a quick look at these two possibilities before landing this? Gijs, FYI: if we go ahead with this patch it should land on inbound instead of UX as it's not something we need to hold back for Australis.
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #6) > Perhaps we could experiment with border-image for the separators instead now > that we've switched to using an image for that since filing this bug. > (Gradients are not supported in border-image in Gecko yet [bug 709587]). > That is easier to maintain compared to the pseudo-elements but may cause new > problems with tab overflow. This seems to be feasible, although this means there's "unclickable" space inbetween the tabs, which is undesirable. I haven't looked at how to fix that, first I'd like to see how win32 and linux look with what I've concocted. > I don't want to hide a warning that is relevant but I also don't really > understand what exactly we're doing that is incorrect XUL. If it is just the > |content| property that is causing an inline text child, then that could > also change to url("chrome://browser/skin/tabbrowser/tab-separator.png") > now. Gijs, can you take a quick look at these two possibilities before > landing this? I can try this as well, it may be easier... > Gijs, FYI: if we go ahead with this patch it should land on inbound instead > of UX as it's not something we need to hold back for Australis. The thing is, we also use (both!) ::before and ::after for (both of!) the sides of selected/hovered tabs themselves. So you still get 4 warnings for each tab you open even if we eliminate the separator ones.
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #7) > (In reply to Matthew N. [:MattN] from comment #6) > > Gijs, FYI: if we go ahead with this patch it should land on inbound instead > > of UX as it's not something we need to hold back for Australis. > > The thing is, we also use (both!) ::before and ::after for (both of!) the > sides of selected/hovered tabs themselves. So you still get 4 warnings for > each tab you open even if we eliminate the separator ones. I've fixed the separators and the background-image case (bug 865698). The one with a gradient background we're better off not fixing (could do url() with a transparent data URI, but discussion on IRC seems to indicate that's unlikely to be more performant than the current solution). So I've still committed this to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/34819ac4912e
Comment 9•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/34819ac4912e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•