Summary: | :before/:after pseudo elements do not always apply to the proper element | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dylan <dylanryan> | ||||||||||||||||
Component: | CSS | Assignee: | Takashi Sakamoto <tasak> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | allan.jensen, cmarcelo, darin, eric, esprehn, hyatt, inferno, macpherson, menard, ojan.autocc, robert, simon.fraser, webkit.review.bot | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | All | ||||||||||||||||||
OS: | All | ||||||||||||||||||
Attachments: |
|
Description
Dylan
2012-08-13 20:26:42 PDT
Created attachment 158196 [details]
Test case for bug
Created attachment 161625 [details]
Patch
Comment on attachment 161625 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161625&action=review > Source/WebCore/css/StyleResolver.cpp:2283 > + // If we have first-letter, before or after pseudo style, do not share this style. > + if (style->hasPseudoStyle(FIRST_LETTER) || style->hasPseudoStyle(BEFORE) || style->hasPseudoStyle(AFTER)) > style->setUnique(); Is removing the optimization the only practical way to fix this problem? Created attachment 161903 [details]
Patch
Comment on attachment 161625 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161625&action=review Thank you for reviewing. >> Source/WebCore/css/StyleResolver.cpp:2283 >> style->setUnique(); > > Is removing the optimization the only practical way to fix this problem? I see. I modified to canShareStyleWithElement to check whether pseudo style can be shared or not. Created attachment 164480 [details]
Patch
Comment on attachment 164480 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164480&action=review > Source/WebCore/ChangeLog:8 > + Disable sharing a style with sibligs if :after or :before pseudo style sibligs => siblings > Source/WebCore/rendering/style/RenderStyle.cpp:264 > + if (!m_cachedPseudoStyles || !m_cachedPseudoStyles->size()) I don't think this size() check is needed since the loop would just never run. Can you remove it and combine this and the below if statement? Created attachment 168626 [details]
Patch
Comment on attachment 164480 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164480&action=review Thank you for reviewing. >> Source/WebCore/ChangeLog:8 >> + Disable sharing a style with sibligs if :after or :before pseudo style > > sibligs => siblings Thanks. Done. >> Source/WebCore/rendering/style/RenderStyle.cpp:264 >> + if (!m_cachedPseudoStyles || !m_cachedPseudoStyles->size()) > > I don't think this size() check is needed since the loop would just never run. Can you remove it and combine this and the below if statement? Sure. Done. Attachment 168626 [details] was posted by a committer and has review+, assigning to Takashi Sakamoto for commit.
Created attachment 184720 [details]
Patch
Since a patch for Bug 101448 (Move childrenAffectedBy bits from RenderStyle to Element) was landed, renamed RenderStyle::pseudoStyleAffectedByUncommonAttributeSelectors to RenderStyle::hasUniquePseudoStyle. Also updated StyleResolver.cpp to use hasUniquePseudoStyle instead of pseudoStyleAffectedByUncommonAttributeSelectors. Created attachment 188980 [details]
Patch for landing
I would like to land this patch in a few days if there are no objections. Comment on attachment 188980 [details] Patch for landing Clearing flags on attachment: 188980 Committed r143300: <http://trac.webkit.org/changeset/143300> All reviewed patches have been landed. Closing bug. |