Summary: | ENABLE_SHADOW_DOM should be available via build-webkit --shadow-dom | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||||||||||
Component: | DOM | Assignee: | Hajime Morrita <morrita> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | dglazkov, dominicc, gustavo, hayato, mrobinson, ossy, pnormand, rakuco, vestbo, webkit.review.bot, xan.lopez, zimmermann | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 63601 | ||||||||||||||||
Attachments: |
|
Description
Hajime Morrita
2012-01-23 14:42:24 PST
As a side note, here is for chromium: http://code.google.com/p/chromium/issues/detail?id=111112 Created attachment 123642 [details]
WIP
This patch intentionally ENABLE_SHADOW_DOM to see it can be built. Land-able version would disable it by default. Note that for CMake-based ports you should add the respective WEBKIT_FEATURE() call in Source/cmake/Options{Efl,Blackberry,WinCE}.cmake. Please add the corresponding update to Tools/qmake/mkspecs/features/features.prf (for the Qt port), and update build-webkit (adding it to the list of enable/disable-options). Created attachment 123756 [details]
WIP v2. Added Qt and CMake flags
Thanks. Looks fine from a Qt perspective. (In reply to comment #7) > Thanks. Looks fine from a Qt perspective. Thank you too for your feedback! Comment on attachment 123756 [details] WIP v2. Added Qt and CMake flags Attachment 123756 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11344001 New failing tests: media/audio-garbage-collect.html Comment on attachment 123756 [details] WIP v2. Added Qt and CMake flags View in context: https://bugs.webkit.org/attachment.cgi?id=123756&action=review Use this commit as an example: http://trac.webkit.org/changeset/101323 > Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:109 > +ENABLE_SHADOW_DOM = ENABLE_SHADOW_DOM; This means "on" by default when building from XCode. > Source/WebCore/Configurations/FeatureDefines.xcconfig:109 > +ENABLE_SHADOW_DOM = ENABLE_SHADOW_DOM; Ditto. > Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:109 > +ENABLE_SHADOW_DOM = ENABLE_SHADOW_DOM; Ditto. > Source/WebKit2/Configurations/FeatureDefines.xcconfig:109 > +ENABLE_SHADOW_DOM = ENABLE_SHADOW_DOM; Ditto. > Tools/Scripts/build-webkit:301 > + define => "ENABLE_SHADOW_DOM", default => 1, value => \$shadowDomSupport }, Again, this is "on" by default. Created attachment 123812 [details]
Patch
Dimitri, thanks for taking a look! I shoul've give --no-review for the last patch - It had wrong flags, no ChangeLogs. But this new one is intend to be ready for your review. Could you take a look again? Comment on attachment 123812 [details]
Patch
are you missing the vsprops file changes by any chance?
Created attachment 123846 [details]
Took care of vsprops files
Comment on attachment 123846 [details] Took care of vsprops files Attachment 123846 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11344485 Created attachment 123950 [details]
Patch for landing
Comment on attachment 123950 [details] Patch for landing Rejecting attachment 123950 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/11344587 Created attachment 123975 [details]
Patch for landing
Comment on attachment 123975 [details] Patch for landing Clearing flags on attachment: 123975 Committed r105922: <http://trac.webkit.org/changeset/105922> All reviewed patches have been landed. Closing bug. Reopen because it broke the GTK build. GTK EWS could have report this breakage, but you didn't wait for it, but commit the patch and leave the build broken. Breaking a build and leaving it broken isn't a fair play game. :-/ (In reply to comment #21) > Reopen because it broke the GTK build. GTK EWS could have report this breakage, but you didn't wait for it, but commit the patch and leave the build broken. > Breaking a build and leaving it broken isn't a fair play game. :-/ I admit, I have to share Ossys concerns. When I woke up today, Gtk/Win were not building, all failing with JSShadowRoot related changes. I wonder why you didn't wait for the EWS for a patch like this? Anyhow, I've fixed Qt-win/Gtk builds in r105975. Next is windows. <UserMacro Name="ENABLE_SHADOW_DOM" Value="ENABLE_SHADOW_DOM" PerformEnvironmentSet="true" /> This enabled the feature by default, I changed this to Value="" in r105976. Closing this bug, next time please be more careful watching the bots and/or using EWS extensively. Ossy, Zimmermann, You are totally right. I apologize my lack of presence. I should've been there, or I shouldn't have landed patch when I cannot be available. Especially for this kind of complex change. Regards, And Zimmermann, thanks much for your kind follow up. (In reply to comment #25) > And Zimmermann, thanks much for your kind follow up. Heh, no worries, it happens to any of us at some point :-) You're welcome. |