incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Mueller (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CB-1193) Weinre: add Windows Phone support
Date Mon, 20 Aug 2012 13:26:38 GMT

    [ https://issues.apache.org/jira/browse/CB-1193?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13437847#comment-13437847
] 

Patrick Mueller commented on CB-1193:
-------------------------------------

comments on https://github.com/apache/incubator-cordova-weinre/pull/9/files

----

all files in weinre.build/vendor/webkit need to be placed outside of that file tree.  Problem
is, the build system is set up to copy the Web Inspector files right from svn, right into
weinre.build/vendor. So if we ever update the version of Web Inspector we're using, all these
files will be overwritten.

Suggest creating a new weinre.build/vendor-override directory, and the placing the files in
there.  The "get-vendor" bits of the ant  build files should then be updated to `cp -R` those
files over the top of weinre.build/vendor.

So, the commit will actually have these files in twice; once in weinre.build/vendor and weinre.build/vendor-override.

----

I'd like to see a monkeypatch of `Object.getPrototypeOf`, something like this:

{noformat}
if (!Object.getPrototypeOf) {
    Object.getPrototypeOf = function(object) {
        if (!object.__proto__) throw new Error("This vm does not support __proto__))")
        return object.__proto__
    }
}
{noformat}

It goes somewhere in the target code, presumably very early.  BrowserHacks?

----

overzealous const -> var translation; remove

* weinre.build/vendor/webkit/WebCore/inspector/front-end/SourceJavaScriptTokenizer.js:
* weinre.build/vendor/webkit/WebCore/inspector/front-end/UglifyJS/parse-js.js
* weinre.build/vendor/webkit/WebCore/inspector/front-end/UglifyJS/process.js

----

empty line for:  weinre.web/client/weinre/hacks.js

Did your IDE do this for you?  Prefer to not see these kinds of things, but survivable.

----

weinre.web/modules/weinre/common/HookLib.coffee

Don't spam the console.  I don't think we need to print this message, just change it to return
if @target is undefined.  And the if statement looks like it's indented  incorrectly.

----

weinre.web/modules/weinre/target/BrowserHacks.coffee

Checking the user agent seems fragile.  Unless you can point me to something that says only
IE-based browsers use it, or that none of the other browsers that we  support use it (Android,
iOS, Blackberry, and for good measure B2G or whatever).

The `apply` function is poorly named, since it's used as a class method, and  thus is an invocation
of the method `apply` on a function, and so shadows `Function.prototype.apply`.

And the BrowserHacks usage in Target.coffee doesn't really require this.  Though weinre was
built early-on as a "every module is a class" story, that's not  needed any more.  I'd just
make this a plain old module, remove the class,  make the `internetExplorerHacks` a top-level
function in the module, and do the body of the `apply` function to the body of the module.
 Then Target.coffee will  be changed to just do a "require 'BrowserHacks'" without the apply.

----

weinre.web/modules/weinre/target/CSSStore.coffee

The calculation of implicit looks wrong.  Don't you need to invoke isPropertyImplicit, rather
than just return it?

I wonder if ms actually supports "matchesSelector" now; I think at the time, no one did, so
I never bothered checking for it.  Is now the time?  

----

weinre.web/modules/weinre/target/Console.coffee

That replacement is not exactly correct.  You are replacing a property with a function, so
at least the invocations of the `original` method will need to be updated.

I think I'd suggest replacing it with a defineProperties() invocation, as you did with some
other changes.

                
> Weinre: add Windows Phone support
> ---------------------------------
>
>                 Key: CB-1193
>                 URL: https://issues.apache.org/jira/browse/CB-1193
>             Project: Apache Cordova
>          Issue Type: New Feature
>          Components: weinre
>            Reporter: Sergey Grebnov
>            Assignee: Sergey Grebnov
>              Labels: weinre, windows
>
> Since Windows Phone does not provide any html debug possibilities it will be great to
support it via Weinre remote debugger.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message