cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Mocny <mmo...@chromium.org>
Subject Re: (CB-2600) Mobile Spec cordova.js only works for Android and IOS
Date Wed, 20 Mar 2013 19:35:46 GMT
(and I'll add that they have already been fixed a week ago and patches to
add platform support have been proposed.. much ado about nothing me thinks)


On Wed, Mar 20, 2013 at 3:34 PM, Michal Mocny <mmocny@chromium.org> wrote:

> Yes, the use case is "laziness" but not for the reasons you outline:
>
> mobile-spec project doesn't have cordova.js set as an ignored file in git
> (and in fact, cordova.js does some stuff in there), so if you make changes
> to mobile-spec itself, you have to take care not to add changes to
> cordova.js as part of your commit.  By making this change, I now no longer
> need to modify cordova.js and no longer worry about accidental commits
> and/or manually reverting changes to cordova.js on each mobile-spec change.
>  I asked the list about how other go about solving this issue and no one
> proposed a good answer.
>
> Your concerns about try/catch and lack of platform support in userAgent
> sniffing are warranted but trivial to fix.  I didn't realize it was that
> much of an embarrassment (I also asked repeatedly for feedback to iron out
> the issues earlier), but I'm very sorry for troubling you.
>
> -Michal
>
>
> On Wed, Mar 20, 2013 at 3:20 PM, Andrew Grieve <agrieve@chromium.org>wrote:
>
>> I consider this change a stop-gap until we have a CLI-based mobile-spec
>> app
>> to work with.
>>
>> Before the change, the project is broken by default and requires you to
>> copy in a script for it to work.
>>
>> After the change, it's still broken by default, but now allows you to have
>> a script-per-platform, so that can run the mobile-spec on multiple
>> platforms without mistakenly including the wrong platform's cordova.js in
>> it.
>>
>> E.g. workflow:
>> jake && cp pkg/* ../cordova-mobile-spec
>>
>> instead of
>> jake && cp pkg/cordova.ios.js
>> ../cordova-mobile-spec/cordova-SOME_VERSION.js
>> and having to re-run each time you switch platforms.
>>
>>
>> I think I do recall now that synchronous xhrs throw exceptions when they
>> fail. try/catch addresses this.
>>
>>
>>
>> On Wed, Mar 20, 2013 at 3:12 PM, Braden Shepherdson <braden@chromium.org
>> >wrote:
>>
>> > The correct solution, to me, is mobile-spec as a plugin. Why does it
>> even
>> > have a cordova.js in it?
>> >
>> > I've turned our chrome-spec test suite for the Chrome Apps stuff into a
>> > plugin that just has an <asset src="www" /> in it to copy its whole
>> > contents into the platform www folders when you install it. It works
>> very
>> > well.
>> >
>> > Braden
>> >
>> >
>> > On Wed, Mar 20, 2013 at 2:53 PM, Filip Maj <fil@adobe.com> wrote:
>> >
>> > > Please see CB-2600 for details, but I want to bring this issue back to
>> > the
>> > > list: the cordova.js loader in mobile-spec inspecting userAgent to
>> > > determine what extra cordova.js to load.
>> > >
>> > > Why I'm against it:
>> > >
>> > > 1. It's sole purpose is to pander to laziness. The SINGLE use case it
>> > > exists for is making a change in cordova-js, rebuilding cordova-js,
>> and
>> > > copying cordova.platform.js into a cordova project with mobile-spec as
>> > its
>> > > app, and not requiring a rename of the cordova.platform.js file. As if
>> > > running an extra `mv cordova.platform.js cordova-x.x.x.js` was too
>> much!
>> > >
>> > > 2. Currently we only inspect for the string "Android" in the userAgent
>> > > [1]. FirefoxOS, webos, windows phone platforms, etc. etc. not
>> accounted
>> > > for. Every new platform that comes in needs to be accounted for in the
>> > > test suite.
>> > >
>> > > 3. Failed script loads lead to unreliable behavior on certain
>> platforms.
>> > > See the comment at [2]. So just wrap it in try-catch?
>> > >
>> > > 4. Introduction of a new global flag to completely avoid this
>> behavior?
>> > [3]
>> > >
>> > > This all looks like amateur hour to me for, at best, marginal benefit.
>> > >
>> > > [1]
>> > >
>> https://github.com/apache/cordova-mobile-spec/blob/master/cordova.js#L23
>> > > [2]
>> > >
>> https://github.com/apache/cordova-mobile-spec/blob/master/cordova.js#L32
>> > > [3]
>> > >
>> https://github.com/apache/cordova-mobile-spec/blob/master/cordova.js#L46
>> > >
>> > > On 3/20/13 11:43 AM, "Filip Maj (JIRA)" <jira@apache.org> wrote:
>> > >
>> > > >
>> > > >    [
>> > > >
>> > >
>> >
>> https://issues.apache.org/jira/browse/CB-2600?page=com.atlassian.jira.plug
>> > >
>> >
>> >in.system.issuetabpanels:comment-tabpanel&focusedCommentId=13607986#commen
>> > > >t-13607986 ]
>> > > >
>> > > >Filip Maj commented on CB-2600:
>> > > >-------------------------------
>> > > >
>> > > >I _really really really_ dislike this whole approach. It offers
>> little
>> > to
>> > > >no benefit, and maintaining this function requires reviewing the
>> > > >mobile-spec cordova.js loader with every platform we introduce.
>> > > >
>> > > >I am adamantly against inspecting userAgent for anything we do. This
>> is
>> > > >not in the spirit of the cordova project, let alone a good practice.
>> > > >
>> > > >> Mobile Spec cordova.js only works for Android and IOS
>> > > >> -----------------------------------------------------
>> > > >>
>> > > >>                 Key: CB-2600
>> > > >>                 URL: https://issues.apache.org/jira/browse/CB-2600
>> > > >>             Project: Apache Cordova
>> > > >>          Issue Type: Bug
>> > > >>          Components: mobile-spec
>> > > >>            Reporter: Jeffrey Heifetz
>> > > >>            Assignee: Tim Kim
>> > > >>            Priority: Blocker
>> > > >>   Original Estimate: 12h
>> > > >>  Remaining Estimate: 12h
>> > > >>
>> > > >> Cordova.js attempts to load platform specific JS, but limits to
>> only
>> > > >>android or ios.
>> > > >
>> > > >--
>> > > >This message is automatically generated by JIRA.
>> > > >If you think it was sent incorrectly, please contact your JIRA
>> > > >administrators
>> > > >For more information on JIRA, see:
>> > http://www.atlassian.com/software/jira
>> > >
>> > >
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message