Bug 51247

Summary:

css attribute + child selector not updated properly when attribute change

Product:

WebKit

Reporter:

arno. <a.renevier>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED CONFIGURATION CHANGED

Severity:

Normal

CC:

ahmad.saleem792, bfulgham, eric, hyatt, jamesr, macpherson, morrita, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
testcase	none
patch v1	none
patch v1.2	jamesr: review-
patch v1.3	none
patch v1.4	eric: review-
patch v1.5	none
patch v1.6	none

arno.

Reported 2010-12-17 05:46:21 PST

Created attachment 76872 [details] testcase Hi, I have a css rule with an attribute selector and a child selector. Such as: div[funky] > span { } it works fine except when "funky" attribute is removed by script, the css rule still applies Also, when setting no attribute at load time, and adding the attribute, the css rule will not apply. I tested it with webkit-gtk and chromium on linux. See attached testcase

Attachments
testcase (744 bytes, text/html) 2010-12-17 05:46 PST, arno.	no flags	Details
patch v1 (3.61 KB, patch) 2010-12-25 14:27 PST, arno.	no flags	Details Formatted Diff Diff
patch v1.2 (1.35 KB, patch) 2010-12-29 12:48 PST, arno.	jamesr: review-	Details Formatted Diff Diff
patch v1.3 (4.28 KB, patch) 2011-02-15 14:58 PST, arno.	no flags	Details Formatted Diff Diff
patch v1.4 (4.35 KB, patch) 2011-02-16 01:22 PST, arno.	eric: review-	Details Formatted Diff Diff
patch v1.5 (4.83 KB, patch) 2011-05-25 09:52 PDT, arno.	no flags	Details Formatted Diff Diff
patch v1.6 (4.84 KB, patch) 2011-05-30 04:19 PDT, arno.	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

arno.

Comment 1 2010-12-25 14:27:12 PST

Created attachment 77442 [details] patch v1 Actually, what happen is: when document()->styleSelector()->hasSelectorForAttribute is called in Element::recalcStyleIfNeededAfterAttributeChanged, m_selectorAttrs does not contain the attribute if attribute selector is followed by a relation selector. It probably has something to do with bug #7492 I don't understand why attribute name is added to m_selectorAttrs only if (elementStyle && (!e->isStyledElement() || (!static_cast<StyledElement*>(e)->isMappedAttribute(attr) && attr != typeAttr && attr != readonlyAttr))) In the case of an attribute selector followed by a relation selector, checkOneSelector is called whith a null elementStyle argument, so attribute name is not added to hashset. If I, once an attribute selector is detected, I add attribute name to m_selectorAttrs unconditionally, the testcase works, and it looks like I don't break anything (but I did not manage to run the full layout tests properly). Here is a patch proposal. Feel free to comment on it, so I can work more on it if needed.

Eric Seidel (no email)

Comment 2 2010-12-28 14:06:12 PST

Comment on attachment 77442 [details] patch v1 You need to include the -expected.txt file from your run-webkit-tests run in your patch. :)

arno.

Comment 3 2010-12-29 12:48:48 PST

Created attachment 77639 [details] patch v1.2 same patch with fast/css/bugzilla-51247-expected.txt added

Hajime Morrita

Comment 4 2011-01-27 23:45:16 PST

Is this valid yet? If not so, could you cancel the r?

arno.

Comment 5 2011-02-14 06:40:49 PST

No, bug is still here, and I'm still waiting for a review.

James Robinson

Comment 6 2011-02-15 14:40:42 PST

Comment on attachment 77639 [details] patch v1.2 This doesn't seem right

arno.

Comment 7 2011-02-15 14:58:18 PST

Created attachment 82529 [details] patch v1.3 Sorry, I did not notice the patch was wrong. This one should the correct one.

Simon Fraser (smfr)

Comment 8 2011-02-15 21:30:51 PST

Comment on attachment 82529 [details] patch v1.3 View in context: https://bugs.webkit.org/attachment.cgi?id=82529&action=review > LayoutTests/ChangeLog:9 > + * fast/css/bugzilla-51247-expected.txt: Added. > + * fast/css/bugzilla-51247.html: Added. Please use a better filename for the testcase; something like attribute-child-selector.html.

arno.

Comment 9 2011-02-16 01:22:26 PST

Created attachment 82602 [details] patch v1.4 same pach with bugzilla-51247 replaced by attribute-child-selector

Eric Seidel (no email)

Comment 10 2011-05-23 14:05:30 PDT

Comment on attachment 82602 [details] patch v1.4 View in context: https://bugs.webkit.org/attachment.cgi?id=82602&action=review This seems like a pretty simple patch. We should be able to get hyatt to venture an opinion. I think we should clean up the test a little and then we can press hyatt for review. > LayoutTests/fast/css/attribute-child-selector-expected.txt:5 > +The following line should say SUCCESS in green letters over a white background. > + > +SUCCESS Why do we need to display this in a dumpAsText test? > LayoutTests/fast/css/attribute-child-selector-expected.txt:7 > + PASS getComputedStyle(document.getElementById('bar')).getPropertyValue('color') is 'rgb(0, 128, 0)' Why is there a leading space before the PASS? > LayoutTests/fast/css/attribute-child-selector.html:2 > +<head id="head"> Why? > LayoutTests/fast/css/attribute-child-selector.html:20 > + <div id="console"> </div> I suspect the space in this console is why there is a space before the PASS. > LayoutTests/fast/css/attribute-child-selector.html:28 > + test(); No need to have a test function since you just call it directly.

arno.

Comment 11 2011-05-25 09:52:44 PDT

Created attachment 94798 [details] patch v1.5 (In reply to comment #10) > > > LayoutTests/fast/css/attribute-child-selector-expected.txt:5 > > +The following line should say SUCCESS in green letters over a white background. > > + > > +SUCCESS > > Why do we need to display this in a dumpAsText test? I'm not sure I understand what you mean. What is a dumpAsText test ? In this patch, I've removed "The following line should say SUCCESS in green letters over a white background." paragraph from the test. > > LayoutTests/fast/css/attribute-child-selector.html:2 > > +<head id="head"> > > Why? fixed > > LayoutTests/fast/css/attribute-child-selector.html:20 > > + <div id="console"> </div> > > I suspect the space in this console is why there is a space before the PASS. Yes, that is the reason, it's fixed in this patch. > > LayoutTests/fast/css/attribute-child-selector.html:28 > > + test(); > > No need to have a test function since you just call it directly. thanks, that is also fixed.

Eric Seidel (no email)

Comment 12 2011-05-29 14:41:47 PDT

Comment on attachment 94798 [details] patch v1.5 View in context: https://bugs.webkit.org/attachment.cgi?id=94798&action=review I'm not sure I understand the patch, because I don't really know what the selectorAttrs map does. I'd have to read the file more carefully. > LayoutTests/fast/css/attribute-child-selector.html:23 > +</body> Might want to include js-test-post.js as well here, but this is OK. > Source/WebCore/ChangeLog:13 > +22011-05-20 Ryosuke Niwa <rniwa@webkit.org> You edited this line by mistake.

arno.

Comment 13 2011-05-30 04:19:03 PDT

Created attachment 95335 [details] patch v1.6 > > LayoutTests/fast/css/attribute-child-selector.html:23 > > +</body> > > Might want to include js-test-post.js as well here, but this is OK. Ok, I've added it. > > Source/WebCore/ChangeLog:13 > > +22011-05-20 Ryosuke Niwa <rniwa@webkit.org> > > You edited this line by mistake. fixed.

arno.

Comment 14 2011-07-14 07:12:29 PDT

Ping. Could someone review latest patch please ?

Eric Seidel (no email)

Comment 15 2011-12-21 11:52:15 PST

Comment on attachment 95335 [details] patch v1.6 Seems OK to me.

Eric Seidel (no email)

Comment 16 2011-12-21 15:31:35 PST

I don't think your latest patch applies, so you'll likely need to upload a new one.

WebKit Review Bot

Comment 17 2011-12-21 15:38:05 PST

Comment on attachment 95335 [details] patch v1.6 Rejecting attachment 95335 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2 Last 500 characters of output: ayoutTests/fast/css/attribute-child-selector-expected.txt patching file LayoutTests/fast/css/attribute-child-selector.html patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/css/CSSStyleSelector.cpp Hunk #1 FAILED at 2377. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/css/CSSStyleSelector.cpp.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10997164

Ahmad Saleem

Comment 18 2022-08-05 08:19:29 PDT

I am unable to reproduce this bug using attached test case in Safari 15.6 on macOS 12.5 and upon clicking "toggle" the coloured text change color and doing it across all other browsers as well (Chrome Canary 106 and Firefox Nightly 105). Since all browsers are matching, I am marking it as "RESOLVED CONFIGURATION CHANGED". Please reopen, if I am wrong. Thanks!

Note You need to log in before you can comment on or make changes to this bug.