You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by dpogue <gi...@git.apache.org> on 2015/01/27 05:52:02 UTC

[GitHub] cordova-android pull request: CB-5059 Add a CookieManager abstract...

GitHub user dpogue opened a pull request:

    https://github.com/apache/cordova-android/pull/151

    CB-5059 Add a CookieManager abstraction for pluggable webviews.

    Crosswalk and GeckoView implementations of CordovaWebView can provide their own ICordovaCookieManager implementation for plugins to use.
    
    /cc @infil00p & @agrieve 
    If you think this is the correct way to go about this, I'll make an implementation for the Crosswalk engine and update the FileTransfer plugin.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/dpogue/cordova-android 4.0.x-cookies

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-android/pull/151.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #151
    
----
commit 411f498a270aeebe33b4cca0bfe1d6176bb53d01
Author: Darryl Pogue <dv...@gmail.com>
Date:   2015-01-27T04:45:25Z

    CB-5059 Add a CookieManager abstraction for pluggable webviews.
    
    Crosswalk and GeckoView implementations of CordovaWebView can provide
    their own ICordovaCookieManager implementation for plugins to use.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: CB-5059 Add a CookieManager abstract...

Posted by dpogue <gi...@git.apache.org>.
Github user dpogue commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/151#discussion_r23665411
  
    --- Diff: framework/src/org/apache/cordova/ICordovaCookieManager.java ---
    @@ -0,0 +1,45 @@
    +/*
    +       Licensed to the Apache Software Foundation (ASF) under one
    +       or more contributor license agreements.  See the NOTICE file
    +       distributed with this work for additional information
    +       regarding copyright ownership.  The ASF licenses this file
    +       to you under the Apache License, Version 2.0 (the
    +       "License"); you may not use this file except in compliance
    +       with the License.  You may obtain a copy of the License at
    +
    +         http://www.apache.org/licenses/LICENSE-2.0
    +
    +       Unless required by applicable law or agreed to in writing,
    +       software distributed under the License is distributed on an
    +       "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +       KIND, either express or implied.  See the License for the
    +       specific language governing permissions and limitations
    +       under the License.
    +*/
    +
    +package org.apache.cordova;
    +
    +public interface ICordovaCookieManager {
    --- End diff --
    
    Sounds good. FileTransfer was just using getCookie() as I recall, but I'll go with Andrew's list (minus setThirdPartyCookiesEnabled?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: CB-5059 Add a CookieManager abstract...

Posted by dpogue <gi...@git.apache.org>.
Github user dpogue commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/151#discussion_r23664825
  
    --- Diff: framework/src/org/apache/cordova/ICordovaCookieManager.java ---
    @@ -0,0 +1,45 @@
    +/*
    +       Licensed to the Apache Software Foundation (ASF) under one
    +       or more contributor license agreements.  See the NOTICE file
    +       distributed with this work for additional information
    +       regarding copyright ownership.  The ASF licenses this file
    +       to you under the Apache License, Version 2.0 (the
    +       "License"); you may not use this file except in compliance
    +       with the License.  You may obtain a copy of the License at
    +
    +         http://www.apache.org/licenses/LICENSE-2.0
    +
    +       Unless required by applicable law or agreed to in writing,
    +       software distributed under the License is distributed on an
    +       "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +       KIND, either express or implied.  See the License for the
    +       specific language governing permissions and limitations
    +       under the License.
    +*/
    +
    +package org.apache.cordova;
    +
    +public interface ICordovaCookieManager {
    --- End diff --
    
    My plan was to make it mostly API-compatible with existing CookieManagers so that it would be easier to migrate existing plugins rather than trying to figure out a different API.
    
    One other issue is that the XWalkCookieManager doesn't appear to have setThirdPartyCookiesEnabled(): https://github.com/crosswalk-project/crosswalk/blob/master/runtime/android/core_internal/src/org/xwalk/core/internal/XWalkCookieManager.java
    
    If those aren't concerns (I guess XWalkCordovaCookieManager could just ignore setThirdPartyCookiesEnabled?), then I'll shrink down the interface.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: CB-5059 Add a CookieManager abstract...

Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/151#discussion_r23665332
  
    --- Diff: framework/src/org/apache/cordova/ICordovaCookieManager.java ---
    @@ -0,0 +1,45 @@
    +/*
    +       Licensed to the Apache Software Foundation (ASF) under one
    +       or more contributor license agreements.  See the NOTICE file
    +       distributed with this work for additional information
    +       regarding copyright ownership.  The ASF licenses this file
    +       to you under the Apache License, Version 2.0 (the
    +       "License"); you may not use this file except in compliance
    +       with the License.  You may obtain a copy of the License at
    +
    +         http://www.apache.org/licenses/LICENSE-2.0
    +
    +       Unless required by applicable law or agreed to in writing,
    +       software distributed under the License is distributed on an
    +       "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +       KIND, either express or implied.  See the License for the
    +       specific language governing permissions and limitations
    +       under the License.
    +*/
    +
    +package org.apache.cordova;
    +
    +public interface ICordovaCookieManager {
    --- End diff --
    
    The problem with using Android APIs and adopting them as our own is that we have to support stupid crap that isn't universal.  I'd rather add an API later that we needed than have some extra cruft that we have to test.  We should work on having the bare minimum needed for FileTransfer to work, and if a third-party API uses this, they can then submit a bug or send a pull request to add something to the API.  Adding to an API is an API change, but it's not a breaking one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: CB-5059 Add a CookieManager abstract...

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/151#discussion_r23613974
  
    --- Diff: framework/src/org/apache/cordova/ICordovaCookieManager.java ---
    @@ -0,0 +1,45 @@
    +/*
    +       Licensed to the Apache Software Foundation (ASF) under one
    +       or more contributor license agreements.  See the NOTICE file
    +       distributed with this work for additional information
    +       regarding copyright ownership.  The ASF licenses this file
    +       to you under the Apache License, Version 2.0 (the
    +       "License"); you may not use this file except in compliance
    +       with the License.  You may obtain a copy of the License at
    +
    +         http://www.apache.org/licenses/LICENSE-2.0
    +
    +       Unless required by applicable law or agreed to in writing,
    +       software distributed under the License is distributed on an
    +       "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +       KIND, either express or implied.  See the License for the
    +       specific language governing permissions and limitations
    +       under the License.
    +*/
    +
    +package org.apache.cordova;
    +
    +public interface ICordovaCookieManager {
    --- End diff --
    
    Looks great, but I think it would be wise to shrink the interface at the start. I'm thinking just:
    
    - setCookiesEnabled()
    - setThirdPartyCookiesEnabled()
    - setCookie()
    - getCookie()
    - clearCookies()
    - flush()


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: CB-5059 Add a CookieManager abstract...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-android/pull/151


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org