RESOLVED FIXED 70021
[CSS Regions]Parse @-webkit-region rule
https://bugs.webkit.org/show_bug.cgi?id=70021
Summary [CSS Regions]Parse @-webkit-region rule
Mihnea Ovidenie
Reported 2011-10-13 05:50:38 PDT
Add support for parsing the region styling rule.
Attachments
Patch (24.90 KB, patch)
2011-10-13 07:03 PDT, Mihnea Ovidenie
webkit-ews: commit-queue-
Patch 2 (27.80 KB, patch)
2011-10-13 08:26 PDT, Mihnea Ovidenie
hyatt: review-
hyatt: commit-queue-
Patch 3 (49.04 KB, patch)
2011-10-14 08:41 PDT, Mihnea Ovidenie
no flags
Patch 4 (49.07 KB, patch)
2011-10-17 06:35 PDT, Mihnea Ovidenie
no flags
Patch for landing (53.15 KB, patch)
2011-10-18 03:30 PDT, Mihnea Ovidenie
no flags
Mihnea Ovidenie
Comment 1 2011-10-13 07:03:43 PDT
WebKit Review Bot
Comment 2 2011-10-13 07:07:01 PDT
Attachment 110839 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/css/CSSRegionStyleRule.cpp:81: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 1 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 3 2011-10-13 07:19:57 PDT
WebKit Review Bot
Comment 4 2011-10-13 07:22:24 PDT
Comment on attachment 110839 [details] Patch Attachment 110839 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10054222
Gyuyoung Kim
Comment 5 2011-10-13 07:28:22 PDT
Mihnea Ovidenie
Comment 6 2011-10-13 08:26:55 PDT
Dave Hyatt
Comment 7 2011-10-13 09:51:33 PDT
Comment on attachment 110849 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=110849&action=review Looks good. Some minor nits. > Source/WebCore/css/CSSGrammar.y:784 > + WEBKIT_REGION_STYLE_RULE_SYM WHITESPACE simple_selector_list maybe_space '{' maybe_space block_rule_list save_block { Why limit to simple selectors? Don't you want selector_list instead? I don't think there's any reason to limit to simple selectors here. > Source/WebCore/css/CSSRegionStyleRule.cpp:40 > + , m_lstCSSRules(rules) I'd prefer this be m_ruleList. > Source/WebCore/css/CSSRegionStyleRule.cpp:42 > + for (unsigned idx = 0; idx < m_lstCSSRules->length(); ++idx) You probably copied this from other crufty old CSS code, but fix the variable name to just be "index" > Source/WebCore/css/CSSRegionStyleRule.cpp:51 > + for (unsigned idx = 0; idx < m_lstCSSRules->length(); ++idx) Same here. > Source/WebCore/css/CSSRegionStyleRule.cpp:64 > + // First add the selectors. > + for (CSSSelector* s = m_selectorList.first(); s; s = CSSSelectorList::next(s)) { > + if (s != m_selectorList.first()) > + result += ", "; > + result += s->selectorText(); > + } I'd add a selectorText() helper to CSSSelectorList and put this code there. > Source/WebCore/css/CSSRegionStyleRule.cpp:70 > + for (unsigned idx = 0; idx < m_lstCSSRules->length(); ++idx) { "index"
Mihnea Ovidenie
Comment 8 2011-10-14 08:41:48 PDT
Hajime Morrita
Comment 9 2011-10-16 22:04:59 PDT
Comment on attachment 111018 [details] Patch 3 According to the last comment from Hyatt, this should be good. So I'll r+ this within a few days if nobody comes.
Mihnea Ovidenie
Comment 10 2011-10-17 06:35:57 PDT
Created attachment 111256 [details] Patch 4 Reworked patch to take into account the latest changes into stylesheet area.
WebKit Review Bot
Comment 11 2011-10-17 06:39:15 PDT
Attachment 111256 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 Updating OpenSource From git://git.webkit.org/WebKit 4f1dd53..293b636 master -> origin/master M LayoutTests/inspector/profiler/heap-snapshot-test.js M LayoutTests/inspector/profiler/heap-snapshot-expected.txt M LayoutTests/inspector/profiler/heap-snapshot.html M LayoutTests/ChangeLog M Source/WebCore/ChangeLog M Source/WebCore/bindings/js/ScriptProfiler.h M Source/WebCore/bindings/js/ScriptProfiler.cpp M Source/WebCore/bindings/v8/ScriptProfiler.h M Source/WebCore/bindings/v8/ScriptProfiler.cpp M Source/WebCore/inspector/InspectorController.cpp M Source/WebCore/inspector/InspectorProfilerAgent.cpp M Source/WebCore/inspector/front-end/HeapSnapshotProxy.js M Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js M Source/WebCore/inspector/front-end/RemoteObject.js M Source/WebCore/inspector/front-end/heapProfiler.css M Source/WebCore/inspector/front-end/DetailedHeapshotView.js M Source/WebCore/inspector/front-end/HeapSnapshot.js M Source/WebCore/inspector/InspectorProfilerAgent.h M Source/WebCore/inspector/Inspector.json M Source/WebCore/English.lproj/localizedStrings.js r97611 = 90b24e894ca5e0e4163364f1d34546bb19261dc1 (refs/remotes/trunk) M WebKitLibraries/ChangeLog M WebKitLibraries/win/tools/vsprops/common.vsprops r97612 = 546adcace757980dbbf6edef33b728cb592d10c0 (refs/remotes/trunk) M Source/WebKit/chromium/ChangeLog M Source/WebKit/chromium/src/js/Tests.js r97613 = 293b63602c28878237fa90e06017499037cd2727 (refs/remotes/trunk) First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/trunk. Updating chromium port dependencies using gclient... Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107. Re-trying 'depot_tools/gclient sync' No such file or directory at Tools/Scripts/update-webkit line 104. If any of these errors are false positives, please file a bug against check-webkit-style.
Mihnea Ovidenie
Comment 12 2011-10-17 07:19:03 PDT
(In reply to comment #11) > Attachment 111256 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 > > Updating OpenSource > From git://git.webkit.org/WebKit > 4f1dd53..293b636 master -> origin/master > M LayoutTests/inspector/profiler/heap-snapshot-test.js > M LayoutTests/inspector/profiler/heap-snapshot-expected.txt > M LayoutTests/inspector/profiler/heap-snapshot.html > M LayoutTests/ChangeLog > M Source/WebCore/ChangeLog > M Source/WebCore/bindings/js/ScriptProfiler.h > M Source/WebCore/bindings/js/ScriptProfiler.cpp > M Source/WebCore/bindings/v8/ScriptProfiler.h > M Source/WebCore/bindings/v8/ScriptProfiler.cpp > M Source/WebCore/inspector/InspectorController.cpp > M Source/WebCore/inspector/InspectorProfilerAgent.cpp > M Source/WebCore/inspector/front-end/HeapSnapshotProxy.js > M Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js > M Source/WebCore/inspector/front-end/RemoteObject.js > M Source/WebCore/inspector/front-end/heapProfiler.css > M Source/WebCore/inspector/front-end/DetailedHeapshotView.js > M Source/WebCore/inspector/front-end/HeapSnapshot.js > M Source/WebCore/inspector/InspectorProfilerAgent.h > M Source/WebCore/inspector/Inspector.json > M Source/WebCore/English.lproj/localizedStrings.js > r97611 = 90b24e894ca5e0e4163364f1d34546bb19261dc1 (refs/remotes/trunk) > M WebKitLibraries/ChangeLog > M WebKitLibraries/win/tools/vsprops/common.vsprops > r97612 = 546adcace757980dbbf6edef33b728cb592d10c0 (refs/remotes/trunk) > M Source/WebKit/chromium/ChangeLog > M Source/WebKit/chromium/src/js/Tests.js > r97613 = 293b63602c28878237fa90e06017499037cd2727 (refs/remotes/trunk) > First, rewinding head to replay your work on top of it... > Fast-forwarded master to refs/remotes/trunk. > Updating chromium port dependencies using gclient... > Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. > Re-trying 'depot_tools/gclient sync' > Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. > Re-trying 'depot_tools/gclient sync' > Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. > Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107. > Re-trying 'depot_tools/gclient sync' > No such file or directory at Tools/Scripts/update-webkit line 104. > > > If any of these errors are false positives, please file a bug against check-webkit-style. I am confused by this message :)
Dave Hyatt
Comment 13 2011-10-17 09:12:51 PDT
Comment on attachment 111256 [details] Patch 4 r=me
WebKit Review Bot
Comment 14 2011-10-17 17:57:16 PDT
Comment on attachment 111256 [details] Patch 4 Rejecting attachment 111256 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: bCore/css/CSSSelectorList.cpp patching file Source/WebCore/css/CSSSelectorList.h patching file Source/WebCore/css/CSSStyleSelector.cpp Hunk #2 succeeded at 441 (offset 2 lines). Hunk #3 succeeded at 1989 (offset -46 lines). patching file Source/WebCore/css/CSSStyleSelector.h patching file Source/WebCore/css/StyleBase.h patching file Source/WebCore/css/tokenizer.flex Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'David Hyatt', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10125041
Mihnea Ovidenie
Comment 15 2011-10-18 03:30:56 PDT
Created attachment 111415 [details] Patch for landing
WebKit Review Bot
Comment 16 2011-10-18 04:49:58 PDT
Comment on attachment 111415 [details] Patch for landing Clearing flags on attachment: 111415 Committed r97738: <http://trac.webkit.org/changeset/97738>
WebKit Review Bot
Comment 17 2011-10-18 04:50:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.