[Webkit-unassigned] [Bug 138310] Add parsing for :role()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=138310

Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #240847|review?                     |review-
              Flags|                            |

--- Comment #2 from Benjamin Poulain <benjamin at webkit.org> ---
Comment on attachment 240847
  --> https://bugs.webkit.org/attachment.cgi?id=240847
Patch

First round or review. Sorry for the delay, I have a huge backlog on my review queue :(

The patch looks good but:
-The new selector should be under ENABLE(CSS_SELECTORS_LEVEL4). Sorry I did not mention that by mail, :role() is part of the new selectors we are adding for Level 4 :(
-You should also include test for things that can go wrong, invalid input. For example, :role(), :role()), :role(25), :role(a, b), etc. We need to ensure we do not generate valid CSSSelector for invalid input.
-You should include tests covering uppercase and lowercase. We still need to define how case-sensitivity will work with :role(), in the meantime we should make sure the behavior is tested so that some tests would break when the behavior changes.
-I have recently added the test fast/selectors/invalid-functional-pseudo-class.html for functional pseudo class. Can you add :role() there too and extend the results?

I am a bit surprised you did not need to modify the parser.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141105/a3a93095/attachment-0002.html>