Bug 90298

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 Flags
Proposed Patch
dimich: review-, jianli: commit-queue-
Proposed Patch
abarth: review+, jianli: commit-queue-
Patch after sync
none
Patch with GTK fix none

Description Jian Li 2012-06-29 10:59:19 PDT
In Chromium port, we're working on supporting frameless windows for extensions, that allow the web contents to have complete control of the window. Frameless windows do not have the standard frame and the web contents cover the whole window. Thus we need to provide a way to let the web contents denote which part of the content area can be used to drag the window. For example, the web contents draw the custom titlebar with the icon, title, close button and etc. We want to let the user be able to drag the titlebar in order to move the window around.

WebKit has already implemented the dashboard region support and exposed a special CSS property -webkit-dashboard-region that lets you specify regions for certain purpose, i.e. being excluded from dragging. The syntax of this property is:
     -webkit-dashboard-region: dashboard-region(label geometry-type offset-top offset-right offset-bottom offset-left)

We would like to introduce another CSS property "-webkit-widget-region", that is essentially a synonym of "-webkit-dashboard-region", since we do not want to get confused with dashboard region support in Apple/WebKit. 
     -webkit-widget-region: region(label geometry-type offset-top offset-right offset-bottom offset-left)

Currently the existing dashboard region codes are wrapped inside the feature guard ENABLE(DASHBOARD_SUPPORT). We would like to put this new CSS property in a new feature guard, ENABLE(WIDGET_REGION_SUPPORT) such that "-webkit-dashboard-region" is only provided under ENABLE(DASHBOARD_SUPPORT) as it is now and "-webkit-widget-region" is only exposed under ENABLE(WIDGET_REGION_SUPPORT).
Comment 1 Jian Li 2012-06-29 11:41:40 PDT
Created attachment 150230 [details]
Proposed Patch
Comment 2 Gustavo Noronha (kov) 2012-06-29 12:31:06 PDT
Comment on attachment 150230 [details]
Proposed Patch

Attachment 150230 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13107507
Comment 3 Jian Li 2012-07-02 15:15:28 PDT
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
Comment 4 Jian Li 2012-07-19 10:51:24 PDT
(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 5 Dmitry Titov 2012-07-19 12:03:37 PDT
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...
Comment 6 Jian Li 2012-07-23 13:11:29 PDT
Created attachment 153849 [details]
Proposed Patch

This path made both defines not mutually exclusive.
Comment 7 Adam Barth 2012-07-31 14:31:15 PDT
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.
Comment 8 Jian Li 2012-07-31 15:47:41 PDT
Created attachment 155654 [details]
Patch after sync
Comment 9 Jian Li 2012-07-31 17:41:29 PDT
Created attachment 155691 [details]
Patch with GTK fix
Comment 10 WebKit Review Bot 2012-07-31 17:45:04 PDT
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 11 WebKit Review Bot 2012-08-01 17:14:46 PDT
Comment on attachment 155691 [details]
Patch with GTK fix

Clearing flags on attachment: 155691

Committed r124389: <http://trac.webkit.org/changeset/124389>
Comment 12 Kent Tamura 2012-08-01 19:04:29 PDT
Jian, would you update http://trac.webkit.org/wiki/FeatureFlags please?
Comment 13 Ahmad Saleem 2022-10-05 17:00:44 PDT
This seems to have landed and didn't back out:

https://github.com/WebKit/WebKit/commit/c6b05e305a47e41ab9bde4ffe8e5cb7c43430e29

Marking this as "RESOLVED FIXED". Thanks!
Comment 14 Radar WebKit Bug Importer 2022-10-05 17:01:30 PDT
<rdar://problem/100830051>