We want to change the CSS property from "-webkit-widget-region" to "-webkit-app-region" with the following syntax: -webkit-app-region: drag|no-drag The reason for the renaming is that widget seems to be too vague and we also want to indicate that it is used for web app. In addition, we want to simplify the syntax as above. We will get the bounding information from the element.
Created attachment 164815 [details] Proposed Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 164815 [details] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164815&action=review I think you went a bit too far in make the implementations separate. It seems fine to make the CSS part separate as that part is different between the two, but the stuff in Document looks really silly duplicated. > Source/WebCore/dom/Document.cpp:466 > -#if ENABLE(DASHBOARD_SUPPORT) || ENABLE(WIDGET_REGION) > +#if ENABLE(DASHBOARD_SUPPORT) > , m_hasDashboardRegions(false) > , m_dashboardRegionsDirty(false) > #endif > +#if ENABLE(WIDGET_REGION) > + , m_hasDraggableRegions(false) > + , m_draggableRegionsDirty(false) > +#endif It doesn't make senes to continue sharing this part? > Source/WebCore/dom/Document.h:1060 > +#if ENABLE(WIDGET_REGION) > + void setDraggableRegionsDirty(bool f) { m_draggableRegionsDirty = f; } > + bool draggableRegionsDirty() const { return m_draggableRegionsDirty; } > + bool hasDraggableRegions () const { return m_hasDraggableRegions; } > + void setHasDraggableRegions(bool f) { m_hasDraggableRegions = f; } > + const Vector<DraggableRegionValue>& draggableRegions() const; > + void setDraggableRegions(const Vector<DraggableRegionValue>&); > +#endif It seems silly to duplicate all of this.
(In reply to comment #3) > (From update of attachment 164815 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=164815&action=review > > I think you went a bit too far in make the implementations separate. It seems fine to make the CSS part separate as that part is different between the two, but the stuff in Document looks really silly duplicated. > > > Source/WebCore/dom/Document.cpp:466 > > -#if ENABLE(DASHBOARD_SUPPORT) || ENABLE(WIDGET_REGION) > > +#if ENABLE(DASHBOARD_SUPPORT) > > , m_hasDashboardRegions(false) > > , m_dashboardRegionsDirty(false) > > #endif > > +#if ENABLE(WIDGET_REGION) > > + , m_hasDraggableRegions(false) > > + , m_draggableRegionsDirty(false) > > +#endif > > It doesn't make senes to continue sharing this part? > > > Source/WebCore/dom/Document.h:1060 > > +#if ENABLE(WIDGET_REGION) > > + void setDraggableRegionsDirty(bool f) { m_draggableRegionsDirty = f; } > > + bool draggableRegionsDirty() const { return m_draggableRegionsDirty; } > > + bool hasDraggableRegions () const { return m_hasDraggableRegions; } > > + void setHasDraggableRegions(bool f) { m_hasDraggableRegions = f; } > > + const Vector<DraggableRegionValue>& draggableRegions() const; > > + void setDraggableRegions(const Vector<DraggableRegionValue>&); > > +#endif > > It seems silly to duplicate all of this. Should we still keep calling these methods as ***dashboardRegions*** in Document for both dashboard region and draggable region? DraggableRegionValue is different from DashboardRegionValue since DraggableRegionValue doesn't need label and clip as in DashboardRegionValue while it adds its own draggable field. In the future, we might add other fields to DraggableRegionValue to support complex shape and z-index if needed.
> Should we still keep calling these methods as ***dashboardRegions*** in Document for both dashboard region and draggable region? I'd probably name them something that makes sense for both. DraggableRegion seems like a reasonable choice. > DraggableRegionValue is different from DashboardRegionValue since DraggableRegionValue doesn't need label and clip as in DashboardRegionValue while it adds its own draggable field. In the future, we might add other fields to DraggableRegionValue to support complex shape and z-index if needed. That fine. We don't need to be able to compile in support for both of them yet. We can just have DraggableRegionValue have different fields depending on which compile flag you've set.
(In reply to comment #5) > > Should we still keep calling these methods as ***dashboardRegions*** in Document for both dashboard region and draggable region? > > I'd probably name them something that makes sense for both. DraggableRegion seems like a reasonable choice. > > > DraggableRegionValue is different from DashboardRegionValue since DraggableRegionValue doesn't need label and clip as in DashboardRegionValue while it adds its own draggable field. In the future, we might add other fields to DraggableRegionValue to support complex shape and z-index if needed. > > That fine. We don't need to be able to compile in support for both of them yet. We can just have DraggableRegionValue have different fields depending on which compile flag you've set. (In reply to comment #5) > > Should we still keep calling these methods as ***dashboardRegions*** in Document for both dashboard region and draggable region? > > I'd probably name them something that makes sense for both. DraggableRegion seems like a reasonable choice. If we want to share the codes for both features, how about something that is different from both DashboardRegion and DraggableRegion such that we will not confuse these two regions, like AnnonatedRegion or TaggedRegion? > > > DraggableRegionValue is different from DashboardRegionValue since DraggableRegionValue doesn't need label and clip as in DashboardRegionValue while it adds its own draggable field. In the future, we might add other fields to DraggableRegionValue to support complex shape and z-index if needed. > > That fine. We don't need to be able to compile in support for both of them yet. We can just have DraggableRegionValue have different fields depending on which compile flag you've set. We do have both feature guards turned on for Mac WebKit such that we can run tests for both features, though Safari or Chromium would turn on just one.
Do you intent to submit the "app-region" CSS property for standardization?
(In reply to comment #7) > Do you intent to submit the "app-region" CSS property for standardization? Yes, we have plan to do so. Thanks.
Created attachment 165875 [details] Proposed Patch Changed as suggested in order to shared rendering update code for both features.
Comment on attachment 165875 [details] Proposed Patch Thanks, this looks much better.
Committed as http://trac.webkit.org/changeset/130829.