CSSStyleRule::setSelectorText() is currently not implemented but is beneficial at least for changing rule selectors via the Web Inspector. Selector source range extraction is needed for the text-based CSS editing.
One thing I am unsure about is the ExceptionCodes I should return from setSelectorText for when there is no document associated with the CSSStyleRule being processed (which CSSParser needs to parse the selector) and there is an error parsing the selector. None of the w3c documents seem to cover this issue. I have chosen the semantically similar WRONG_DOCUMENT_ERR and SYNTAX_ERR, but there may be another opinion.
Created attachment 68541 [details] [PATCH] Suggested solution
Attachment 68541 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/css/CSSParser.cpp:5750: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 68544 [details] [PATCH] Style fixed
Actually, per http://dev.w3.org/csswg/cssom/ you should not throw a SYNTAX_ERR (just ignore it). I'm not sure how you can get a CSSStyleRule separately from a style sheet.
(In reply to comment #5) > Actually, per http://dev.w3.org/csswg/cssom/ you should not throw a SYNTAX_ERR (just ignore it). I'm not sure how you can get a CSSStyleRule separately from a style sheet. You are correct. It's just I didn't connect this fact with the usage of CSSStyleRule::setSelectorText in JSCSSStyleRule.cpp. Anyway, if I implement it to always return "no error", it will be hard to tell if the operation has succeeded (e.g. due to a wrong selector list syntax).
Created attachment 68661 [details] [PATCH] Do not return an ExceptionCode in the event of an error in setSelectorText()
Error handling fixed, I've got another question: is doc->styleSelectorChanged(RecalcStyleImmediately); the right way to notify the document of the rule selector change?
You can tell if setting succeeded by comparing the new string with the old string. A lot of web platform APIs taking strings work this way.
(In reply to comment #9) > You can tell if setting succeeded by comparing the new string with the old string. A lot of web platform APIs taking strings work this way. That's right - I didn't mean to say it was impossible (I said "hard" :)). But since I'm working directly with the CSSStyleRule native instance, I'd like to have a simple means of detecting the operation success/failure. I believe the API was dictated by the IDL and not intended to be interacted with from the native code. On a related note, you must know that the selectorText setter previously used to raise DOMException, as per http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSStyleRule, not unreasonably, I should say :)
(In reply to comment #9) > You can tell if setting succeeded by comparing the new string with the old string. A lot of web platform APIs taking strings work this way. Ah, one more note. This approach will likely fail when setting the same selector (an idempotent operation), since the serialized selector list is canonicalized in the sense that extraneous whitespace is removed. Thus, "div span" (one space) and "div span" (two spaces) correspond to the same selector, but cssRule.selectorText will return "div span" in both cases making one think that the operation has failed.
Comment on attachment 68661 [details] [PATCH] Do not return an ExceptionCode in the event of an error in setSelectorText() This should be broken into two at least two patches, one for the implementation of setSelectorText with tests, and one for the rest of the patch.
Created attachment 68918 [details] [PATCH] Only source offsets extraction for rule selectors
Created attachment 69031 [details] [PATCH] Fix selector list end detection
Created attachment 69794 [details] [PATCH] Rebase + small API change
Attachment 69794 [details] did not build on qt: Build output: http://queues.webkit.org/results/4245052
Attachment 69794 [details] did not build on mac: Build output: http://queues.webkit.org/results/4150081
Created attachment 69804 [details] [PATCH] Rebase for compilability
What is the UI for this?
(In reply to comment #19) > What is the UI for this? There are no changes to the UI. The ultimate goal is to add support for style properties not supported by WebKit (moz- and such) without changing the style editing UI. We will also get persistence for local modifications and such. We are fairly close to switching to it!
Comment on attachment 69804 [details] [PATCH] Rebase for compilability View in context: https://bugs.webkit.org/attachment.cgi?id=69804&action=review Overall looks good. Few nits from me and we are ready to land. > WebCore/css/CSSParser.cpp:-152 > - , m_currentStyleData(0) Do not remove this line. You could access style data via rule data.
Created attachment 69938 [details] [PATCH] Comment addressed
Committed: http://trac.webkit.org/changeset/69196