Summary: | Add new CSS property "-webkit-widget-region" to expose dashboard region support for other port | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jian Li <jianli> | ||||||||||
Component: | WebCore Misc. | Assignee: | Jian Li <jianli> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, ahmad.saleem792, bdakin, cmarcelo, darin, dimich, eric, gustavo, gyuyoung.kim, hyatt, macpherson, menard, mifenton, mitz, mrowe, philn, rakuco, tkent, webkit-bug-importer, webkit.review.bot, xan.lopez | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Jian Li
2012-06-29 10:59:19 PDT
Created attachment 150230 [details]
Proposed Patch
Comment on attachment 150230 [details] Proposed Patch Attachment 150230 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13107507 Darin, could you please take a look at this patch? Or if you know someone else who can review this, please let me know. Thanks. I've sent our proposal to webkit-dev. So far I have not received any objection. http://lists.webkit.org/pipermail/webkit-dev/2012-June/021257.html (In reply to comment #3) > Darin, could you please take a look at this patch? Or if you know someone else who can review this, please let me know. Thanks. > > I've sent our proposal to webkit-dev. So far I have not received any objection. > http://lists.webkit.org/pipermail/webkit-dev/2012-June/021257.html Darin, any comment on this patch? Thanks. Comment on attachment 150230 [details] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150230&action=review I quickly looked at the patch itself... It seems it doesn't build on GTK (see EWS failure). Also, it adds a CSS property but does not have tests that verify that it is actually exposed. Could you add a test or is the plan to not enable the feature now and enable it in builds with a test as a subsequent patch? It seems ok to use the existing support that was implemented for Dashboard Widgets (if not, I hope Apple folks will chime in) - but it seems, on a cursory look, that test support for this is thin. If we are to use this mechanism, we'll need to add way more layout tests. Can be done in other patches, of course. > Source/WebCore/css/CSSParser.cpp:4252 > +#if ENABLE(DASHBOARD_SUPPORT) This makes DASHBOARD_SUPPORT and WIDGET_REGION mutually exclusive, when they should not be, or we can't have a build with both enabled... Created attachment 153849 [details]
Proposed Patch
This path made both defines not mutually exclusive.
Comment on attachment 153849 [details]
Proposed Patch
Ok. Once the System Applications working group finishes booting up, we should take this proposal to the CSS working group.
Created attachment 155654 [details]
Patch after sync
Created attachment 155691 [details]
Patch with GTK fix
Comment on attachment 155654 [details] Patch after sync Cleared Adam Barth's review+ from obsolete attachment 155654 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 155691 [details] Patch with GTK fix Clearing flags on attachment: 155691 Committed r124389: <http://trac.webkit.org/changeset/124389> Jian, would you update http://trac.webkit.org/wiki/FeatureFlags please? This seems to have landed and didn't back out: https://github.com/WebKit/WebKit/commit/c6b05e305a47e41ab9bde4ffe8e5cb7c43430e29 Marking this as "RESOLVED FIXED". Thanks! |