WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
88645
[CSS Regions] Adding feature defines for CSS Regions for Windows
https://bugs.webkit.org/show_bug.cgi?id=88645
Summary
[CSS Regions] Adding feature defines for CSS Regions for Windows
Mihai Balan
Reported
2012-06-08 04:38:57 PDT
The fix for #87442 added feature defines for CSS Regions and exclusions to provide proper disabling of these features. However, the proper project defines were added only to the Mac projects, effectively disabling CSS Regions & exclusions on Windows nightlies.
Attachments
Added proper values to ENABLE_CSS_REGIONS & ENABLE_CSS_EXCLUSIONS in FeatureDefines.vsprops and FeatureDefinesCairo.vsprops
(2.27 KB, patch)
2012-06-08 06:02 PDT
,
Mihai Balan
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Another attempt at fixing regions on Windows.
(3.61 KB, patch)
2012-06-11 06:58 PDT
,
Mihai Balan
no flags
Details
Formatted Diff
Diff
FIxed style
(3.67 KB, patch)
2012-06-11 07:07 PDT
,
Mihai Balan
no flags
Details
Formatted Diff
Diff
Pretty much the same as the initial patch, but this time I touched CSSPropertyNames.in to trigger a proper rebuild on Windows.
(3.80 KB, patch)
2012-06-13 11:48 PDT
,
Mihai Balan
no flags
Details
Formatted Diff
Diff
Enabling only regions this time. Also with additions to the settings part and a updated expectations for a couple of tests
(12.00 KB, patch)
2012-06-26 10:00 PDT
,
Mihai Balan
no flags
Details
Formatted Diff
Diff
Fixed style although I'm not sure that removing the parameter name in WebPreferences.h was the right way to do it.
(11.99 KB, patch)
2012-06-26 10:17 PDT
,
Mihai Balan
eric
: review+
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cq-03
(573.60 KB, application/zip)
2012-06-26 15:04 PDT
,
WebKit Review Bot
no flags
Details
Archive of layout-test-results from ec2-cr-linux-03
(596.16 KB, application/zip)
2012-06-26 17:53 PDT
,
WebKit Review Bot
no flags
Details
Fixing layout test failures on cr-linux
(72.78 KB, patch)
2012-06-27 03:37 PDT
,
Mihai Balan
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cr-linux-02
(584.79 KB, application/zip)
2012-06-27 06:09 PDT
,
WebKit Review Bot
no flags
Details
Archive of layout-test-results from ec2-cr-linux-04
(614.14 KB, application/zip)
2012-06-27 07:11 PDT
,
WebKit Review Bot
no flags
Details
Added chromium expectations too, since it seems it first falls through mac expectations :(
(88.15 KB, patch)
2012-06-27 07:17 PDT
,
Mihai Balan
tony
: review-
tony
: commit-queue-
Details
Formatted Diff
Diff
Removed expectations files to play nicely with property-names.js
(11.48 KB, patch)
2012-06-29 09:59 PDT
,
Mihai Balan
no flags
Details
Formatted Diff
Diff
Removed expectation files from ChangeLog, too.
(10.47 KB, patch)
2012-06-29 10:07 PDT
,
Mihai Balan
no flags
Details
Formatted Diff
Diff
Fixed copy/paste error in Source/WebKit/win/WebPreferenceKeysPrivate.h
(10.47 KB, patch)
2012-06-29 10:09 PDT
,
Mihai Balan
tony
: review+
Details
Formatted Diff
Diff
Fixed patch. Previous was hand-edited and had some inconsistencies.
(10.47 KB, patch)
2012-06-29 10:25 PDT
,
Mihai Balan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(11)
View All
Add attachment
proposed patch, testcase, etc.
Mihai Balan
Comment 1
2012-06-08 06:02:15 PDT
Created
attachment 146547
[details]
Added proper values to ENABLE_CSS_REGIONS & ENABLE_CSS_EXCLUSIONS in FeatureDefines.vsprops and FeatureDefinesCairo.vsprops
Build Bot
Comment 2
2012-06-08 07:23:30 PDT
Comment on
attachment 146547
[details]
Added proper values to ENABLE_CSS_REGIONS & ENABLE_CSS_EXCLUSIONS in FeatureDefines.vsprops and FeatureDefinesCairo.vsprops
Attachment 146547
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/12917502
Tim Horton
Comment 3
2012-06-08 07:41:49 PDT
See
https://bugs.webkit.org/show_bug.cgi?id=87519
, where I had the same realization and then the same issue and then never returned to test it manually and figure out what was up.
Mihai Balan
Comment 4
2012-06-11 06:58:59 PDT
Created
attachment 146850
[details]
Another attempt at fixing regions on Windows.
WebKit Review Bot
Comment 5
2012-06-11 07:03:09 PDT
Attachment 146850
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:10: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mihai Balan
Comment 6
2012-06-11 07:07:34 PDT
Created
attachment 146851
[details]
FIxed style
Tim Horton
Comment 7
2012-06-11 09:38:20 PDT
Comment on
attachment 146851
[details]
FIxed style View in context:
https://bugs.webkit.org/attachment.cgi?id=146851&action=review
> Source/WebCore/css/CSSPropertyNames.in:370 > +#if defined(ENABLE_CSS_REGIONS)
Won't this do the wrong thing if it's defined to 0? Is that a case we can get into? I assumed there was a reason for this construct (which is used in many places).
Mihai Balan
Comment 8
2012-06-13 11:48:05 PDT
Created
attachment 147377
[details]
Pretty much the same as the initial patch, but this time I touched CSSPropertyNames.in to trigger a proper rebuild on Windows.
Tim Horton
Comment 9
2012-06-13 15:47:57 PDT
***
Bug 87519
has been marked as a duplicate of this bug. ***
WebKit Review Bot
Comment 10
2012-06-13 22:23:01 PDT
Comment on
attachment 147377
[details]
Pretty much the same as the initial patch, but this time I touched CSSPropertyNames.in to trigger a proper rebuild on Windows. Clearing flags on attachment: 147377 Committed
r120280
: <
http://trac.webkit.org/changeset/120280
>
WebKit Review Bot
Comment 11
2012-06-13 22:23:06 PDT
All reviewed patches have been landed. Closing bug.
Jon Honeycutt
Comment 12
2012-06-15 15:23:50 PDT
This change is causing all layout tests to crash on Windows.
WebKit Review Bot
Comment 13
2012-06-15 21:15:57 PDT
Re-opened since this is blocked by 89273
Mihai Balan
Comment 14
2012-06-26 10:00:38 PDT
Created
attachment 149548
[details]
Enabling only regions this time. Also with additions to the settings part and a updated expectations for a couple of tests
WebKit Review Bot
Comment 15
2012-06-26 10:09:29 PDT
Attachment 149548
[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/WebKit/win/WebPreferences.h:166: Extra space after ( in function call [whitespace/parens] [4] Source/WebKit/win/WebPreferences.h:169: Extra space after ( in function call [whitespace/parens] [4] Source/WebKit/win/WebPreferences.h:170: The parameter name "enabled" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/win/WebPreferences.cpp:912: Extra space after ( in function call [whitespace/parens] [4] Source/WebKit/win/WebPreferences.cpp:919: Extra space after ( in function call [whitespace/parens] [4] Total errors found: 5 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mihai Balan
Comment 16
2012-06-26 10:17:43 PDT
Created
attachment 149553
[details]
Fixed style although I'm not sure that removing the parameter name in WebPreferences.h was the right way to do it.
Eric Seidel (no email)
Comment 17
2012-06-26 11:02:10 PDT
Comment on
attachment 149553
[details]
Fixed style although I'm not sure that removing the parameter name in WebPreferences.h was the right way to do it. View in context:
https://bugs.webkit.org/attachment.cgi?id=149553&action=review
> Source/WebCore/css/CSSPropertyNames.in:9 > +//
Please file a bug against whatever build system needs this. :(
Eric Seidel (no email)
Comment 18
2012-06-26 11:03:01 PDT
The change technically looks fine. I've CC'd Apple Windows folk in case they don't want CSS regions on for their port for some reason.
Mihai Balan
Comment 19
2012-06-26 11:09:30 PDT
They are OK with CSS Regions on Windows, but initially they had a problem when enabling regions AND exclusions. The problem with exclusions is still there and it's pretty hairy. This patch only adds regions support. So unless I break something (again), they should be ok with it :) But it's good to have them cc'ed, too.
WebKit Review Bot
Comment 20
2012-06-26 15:04:35 PDT
Comment on
attachment 149553
[details]
Fixed style although I'm not sure that removing the parameter name in WebPreferences.h was the right way to do it. Rejecting
attachment 149553
[details]
from commit-queue. New failing tests: fast/css/getComputedStyle/computed-style-without-renderer.html fast/css/getComputedStyle/computed-style.html Full output:
http://queues.webkit.org/results/13098575
WebKit Review Bot
Comment 21
2012-06-26 15:04:45 PDT
Created
attachment 149611
[details]
Archive of layout-test-results from ec2-cq-03 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
WebKit Review Bot
Comment 22
2012-06-26 17:53:25 PDT
Comment on
attachment 149553
[details]
Fixed style although I'm not sure that removing the parameter name in WebPreferences.h was the right way to do it.
Attachment 149553
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/13103576
New failing tests: fast/css/getComputedStyle/computed-style-without-renderer.html fast/css/getComputedStyle/computed-style.html
WebKit Review Bot
Comment 23
2012-06-26 17:53:30 PDT
Created
attachment 149648
[details]
Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Mihai Balan
Comment 24
2012-06-27 03:37:24 PDT
Created
attachment 149718
[details]
Fixing layout test failures on cr-linux
WebKit Review Bot
Comment 25
2012-06-27 06:09:07 PDT
Comment on
attachment 149718
[details]
Fixing layout test failures on cr-linux
Attachment 149718
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/13096762
New failing tests: fast/css/getComputedStyle/computed-style-without-renderer.html fast/css/getComputedStyle/computed-style.html
WebKit Review Bot
Comment 26
2012-06-27 06:09:12 PDT
Created
attachment 149737
[details]
Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
WebKit Review Bot
Comment 27
2012-06-27 07:11:44 PDT
Comment on
attachment 149718
[details]
Fixing layout test failures on cr-linux
Attachment 149718
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/13087863
New failing tests: fast/css/getComputedStyle/computed-style-without-renderer.html fast/css/getComputedStyle/computed-style.html
WebKit Review Bot
Comment 28
2012-06-27 07:11:49 PDT
Created
attachment 149743
[details]
Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Mihai Balan
Comment 29
2012-06-27 07:17:45 PDT
Created
attachment 149744
[details]
Added chromium expectations too, since it seems it first falls through mac expectations :(
Eric Seidel (no email)
Comment 30
2012-06-27 07:48:04 PDT
Comment on
attachment 149744
[details]
Added chromium expectations too, since it seems it first falls through mac expectations :( I'm confused why so many computed-style results are needed here? You're just turning this on for Windows, no? Alos, I thought Arv recently removed/made-consistent the computed-style tests?
Mihai Balan
Comment 31
2012-06-27 08:01:26 PDT
Yes, but I want to make sure that it passes on all ports that have regions enabled. I haven't seen any changes to make the computed style tests aware of regions in the initial patch. So I thought this was the right way to do it. Maybe I'm missing something, I feel like I'm playing with things way above my head :(
Eric Seidel (no email)
Comment 32
2012-06-27 08:10:35 PDT
Comment on
attachment 149744
[details]
Added chromium expectations too, since it seems it first falls through mac expectations :( Please run "webkit-patch optimize-baselines fast/css/getComputedStyle/computed-style.html" locally and re-upload. I expect you'll find you don't need this many copies of computed-style-expected.txt
Erik Arvidsson
Comment 33
2012-06-27 13:37:55 PDT
(In reply to
comment #30
)
> (From update of
attachment 149744
[details]
) > I'm confused why so many computed-style results are needed here? You're just turning this on for Windows, no? Alos, I thought Arv recently removed/made-consistent the computed-style tests?
I didn't remove the computed style tests. Ojan said he was going to remove that one...
Ojan Vafai
Comment 34
2012-06-27 13:48:44 PDT
(In reply to
comment #33
)
> (In reply to
comment #30
) > > (From update of
attachment 149744
[details]
[details]) > > I'm confused why so many computed-style results are needed here? You're just turning this on for Windows, no? Alos, I thought Arv recently removed/made-consistent the computed-style tests? > > I didn't remove the computed style tests. Ojan said he was going to remove that one...
Might do it one day, but not anytime soon. If someone else wants to do this, go for it.
Tony Chang
Comment 35
2012-06-27 14:18:27 PDT
(In reply to
comment #34
)
> (In reply to
comment #33
) > > (In reply to
comment #30
) > > > (From update of
attachment 149744
[details]
[details] [details]) > > > I'm confused why so many computed-style results are needed here? You're just turning this on for Windows, no? Alos, I thought Arv recently removed/made-consistent the computed-style tests? > > > > I didn't remove the computed style tests. Ojan said he was going to remove that one... > > Might do it one day, but not anytime soon. If someone else wants to do this, go for it.
I got rid of the platform specific results in
http://trac.webkit.org/changeset/120870
. They're not in ToT which is why it looks like you're adding the files in the diff.
Mihai Balan
Comment 36
2012-06-29 08:30:38 PDT
(In reply to
comment #35
)
> (In reply to
comment #34
) > > (In reply to
comment #33
) > > > (In reply to
comment #30
) > > > > (From update of
attachment 149744
[details]
[details] [details] [details]) > > > > I'm confused why so many computed-style results are needed here? You're just turning this on for Windows, no? Alos, I thought Arv recently removed/made-consistent the computed-style tests? > > > > > > I didn't remove the computed style tests. Ojan said he was going to remove that one... > > > > Might do it one day, but not anytime soon. If someone else wants to do this, go for it. > > I got rid of the platform specific results in
http://trac.webkit.org/changeset/120870
. They're not in ToT which is why it looks like you're adding the files in the diff.
Tony, could you give me some details regarding the r-?
Mihai Balan
Comment 37
2012-06-29 09:59:29 PDT
Created
attachment 150202
[details]
Removed expectations files to play nicely with property-names.js
Mihai Balan
Comment 38
2012-06-29 10:07:17 PDT
Created
attachment 150205
[details]
Removed expectation files from ChangeLog, too.
Mihai Balan
Comment 39
2012-06-29 10:09:44 PDT
Created
attachment 150206
[details]
Fixed copy/paste error in Source/WebKit/win/WebPreferenceKeysPrivate.h
Tony Chang
Comment 40
2012-06-29 10:14:03 PDT
Comment on
attachment 150206
[details]
Fixed copy/paste error in Source/WebKit/win/WebPreferenceKeysPrivate.h It would be nice to have the win EWS bot cycle green before landing.
Mihai Balan
Comment 41
2012-06-29 10:25:13 PDT
Created
attachment 150213
[details]
Fixed patch. Previous was hand-edited and had some inconsistencies.
Tony Chang
Comment 42
2012-06-29 10:36:59 PDT
Comment on
attachment 150213
[details]
Fixed patch. Previous was hand-edited and had some inconsistencies. I'll cq+ after the win EWS bot goes green.
WebKit Review Bot
Comment 43
2012-06-29 15:51:23 PDT
Comment on
attachment 150213
[details]
Fixed patch. Previous was hand-edited and had some inconsistencies. Clearing flags on attachment: 150213 Committed
r121597
: <
http://trac.webkit.org/changeset/121597
>
WebKit Review Bot
Comment 44
2012-06-29 15:51:31 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.
Top of Page
Format For Printing
XML
Clone This Bug