Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#18168 closed defect (fixed)

support iOS8

Reported by: Patrick Ruzand Owned by: Adrian Vasiliu
Priority: high Milestone: 1.7.7
Component: DojoX Mobile Version: 1.10.0
Keywords: ios8 Cc: Adrian Vasiliu, cjolif
Blocked By: Blocking:

Description

The purpose of this ticket is to track all the issues reported against dojo mobile during iOS8 beta tests.

Change History (45)

comment:1 Changed 5 years ago by Patrick Ruzand

Cc: Adrian Vasiliu added
Keywords: ios8 added

comment:2 Changed 5 years ago by cjolif

Cc: cjolif added

comment:3 Changed 5 years ago by cjolif

Priority: undecidedhigh

comment:4 Changed 5 years ago by jmcl1

Hi

Issues I am seeing on our mobile app, running 1.9.0

Lots of areas in the framework are attempting to access or change Event objects which are iOS8 seems to have made read only messages such as "Deprecated attempt to access property 'changedTouches' on a non-TouchEvent? object." are seen.

One example of where touch events are being manipulated can be see in "'dojo/on'" and the function fixTouchListener

This is preventing clicks and touches from being enacted.. I'm still trying to work out exactly what part of the function breaks it as some of the click events do get through.

comment:5 Changed 5 years ago by Adrian Vasiliu

Created https://github.com/dojo/dojo/pull/104 to address the event handling issue.

@jmcl1: this pull request is for master but the same fix should be applicable to 1.9 (and 1.10 as well). If you could give it a try on your side, it would be helpful as a double-check (on my side it solves the event handling issue).

Last edited 5 years ago by Adrian Vasiliu (previous) (diff)

comment:6 in reply to:  5 Changed 5 years ago by jmcl1

Replying to Adrian:

Created https://github.com/dojo/dojo/pull/104 to address the event handling issue.

@jmcl1: this pull request is for master but the same fix should be applicable to 1.9 (and 1.10 as well). If you could give it a try on your side, it would be helpful as a double-check (on my side it solves the event handling issue).

Hi Adrian, I will give it a go tonight (need to set up an rss feed on this ticket!) and let you know the outcome.

Joe

comment:7 Changed 5 years ago by Adrian Vasiliu

Thanks @jmcl1. In the meantime, I've pushed the fix into the master, 1.10, and 1.9 branches (https://github.com/dojo/dojo/pull/104#issuecomment-50669962 ).

comment:8 in reply to:  7 ; Changed 5 years ago by jmcl1

Replying to Adrian:

Thanks @jmcl1. In the meantime, I've pushed the fix into the master, 1.10, and 1.9 branches (https://github.com/dojo/dojo/pull/104#issuecomment-50669962 ).

Hi I've taken the patch and applied it to my mobile projects. I'm no longer seeing the above issue and transitions and touch events work correctly.

I'll keep a monitor on this ticket and also re-test with each iOS beta release, posting any updates

Thanks a lot for your help.

comment:9 Changed 5 years ago by Adrian Vasiliu

Great, thanks @jmcl1.

comment:10 Changed 5 years ago by Adrian Vasiliu

Concerning the behavior of dojo/has: has("ios") surprisingly returns 10.94 in the iPhone and iPad simulators of iOS 8 beta 4 (the UA includes the Mac OS X version which runs the simulator instead of the version number of iOS). However, running on real iPad and iPhone devices with iOS 8 beta 4, has("ios") returns 8 as expected.

comment:11 Changed 5 years ago by Adrian Vasiliu

A note about the dojo/on fix: with the fresh new iOS 8 beta 5, the fix still works, and is still necessary.

comment:12 Changed 5 years ago by Adrian Vasiliu

Another misbehavior of dojox/mobile apps concerns the layout issues. It goes as follows:

  • Some severe issues seen with the initial beta(s) are gone at least since beta 4.
  • In Safari beta 4, only on iPhone (not on iPad), ScrollableView?'s fixed footer is misplaced when turning from portrait to landscape. (The fact that it only affects iPhone in landscape is related with the changes in the full-screen management for iPhone in landscape). Side note, these issues would probably not affect a Cordova app, where the full-screen mode is irrelevant.
  • I have searched for a fix/workaround, and found only one based on delaying the call of dojox/mobile/common.resizeAll when receiving orientation change events. This does the trick, but it is a painful workaround.
  • In Safari beta 5, the misplacement when turning from portrait to landscape is gone (great...), but now the initial placement is incorrect when loading the app directly in landscape instead of loading in portrait then turning to landscape (again, this only holds for iPhone).
  • The workaround I found for the issue in beta 4 (now gone in beta 5) doesn't solve the issue which appeared with beta 5...
  • To be on the safe side, it might be worth spending time to search a solution for the new issue but on the other side it might simply go away in a next beta...

comment:13 in reply to:  8 Changed 5 years ago by jmcl1

Replying to jmcl1:

Replying to Adrian:

Thanks @jmcl1. In the meantime, I've pushed the fix into the master, 1.10, and 1.9 branches (https://github.com/dojo/dojo/pull/104#issuecomment-50669962 ).

Hi I've taken the patch and applied it to my mobile projects. I'm no longer seeing the above issue and transitions and touch events work correctly.

I'll keep a monitor on this ticket and also re-test with each iOS beta release, posting any updates

Thanks a lot for your help.

Ok something quite odd has cropped up. Depending on how I run a build for my project I see/ don't see the above issues. It is obviously something weird going on with my set up but only mentioning here in-case others see it. Investigating.

comment:14 Changed 5 years ago by Adrian Vasiliu

Thanks for sharing it anyway. As you, I would think this is likely to be due to some build configuration issues, but you never know... Let us know if you find something. A few things to check:

  • With the built version of Dojo with the patch applied, do you see in webinspector's console the same errors as with the non-built and non-patched Dojo?
  • Can you put, say, an alert() in the patched dojo/on.js file (in the code which executes only once per app life cycle) such that you can check whether it is indeed the patched version which gets included in the build?
  • With the built and patched Dojo, does it work fine on iOS < 8?
  • And what is the Dojo version you work with? The official 1.10 release? 1.9.0 as you mentioned in your first post? (in this case it would be better to switch to the latest patch, 1.9.3) or a clone of one of Dojo's branches?

comment:15 Changed 5 years ago by jmcl1

So it turns out I was doing something very stupid and point to an older version of the dojo source for certain builds. Confirmed working on simulator with 1.9.3

comment:16 Changed 5 years ago by Adrian Vasiliu

Thanks for the good news!

comment:17 Changed 5 years ago by Adrian Vasiliu

Concerning the issue with the misplaced ScrollableView?'s footer on iPhone in landscape orientation: I have verified that the issue does NOT hurt when the webapp is embedded in a Cordova 3.5.0 container.

comment:18 Changed 5 years ago by bill

Milestone: tbd1.10.1

Setting for 1.10.1 as per today's meeting.

comment:19 Changed 5 years ago by Patrick Ruzand

Owner: set to Adrian Vasiliu
Status: newassigned

comment:20 Changed 5 years ago by Sherbrooke Andrews

Looks like there is an issue with the latest beta build -

(new EventDelegate?).target = null;

doesn't throw an error

(new EventDelegate?()).target = null;

does however

comment:21 Changed 5 years ago by Adrian Vasiliu

Could you please be more specific? Do you observe a misbehavior when using the current patched version of dojo/on in dojo's master or 1.10/1.9 branches? If yes, please provide details (a way to reproduce).

The variant (new EventDelegate()).target = null; is not present in the patched dojo/on, and users cannot write such code because EventDelegate is only available internally.

By "latest beta build" you mean iOS 8 beta 5, right?

comment:22 Changed 5 years ago by Sherbrooke Andrews

yes iOS beta 5 - iOS 8.0 (12A4345d)

I'm using the 1.9 file from https://github.com/dojo/dojo/tree/1.9, spicificly talking about https://github.com/dojo/dojo/blob/1.9/on.js line 24

this dosent work

(new EventDelegate).target = null;
return true;

it dosent throw an error and the .target property of the EventDelegate? is still set to undefined (not null)

I've replaced it with the following in my local version

var b=new a;
b.target=null;
return null===b.target

Example code (Note: I don't have a public environment I can put this up on)-

<!DOCTYPE html>
<html>
<head>
	<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=0">
	<script src="dojox/mobile/deviceTheme.js"></script>
</head>
<body style="visibility: hidden;">
<div data-dojo-type="dojox/mobile/View">
	<div data-dojo-type="dojox/mobile/RoundRectList">
		<div data-dojo-props="href: 'http://cnn.com'" data-dojo-type="dojox/mobile/ListItem">Go to CNN - dosent work with existing dojo/on code</div>
	</div>
	<span id="clickBtn" style="display: inline-block; padding: 10px; background-color: grey;">Click me, this button always works</span>
</div>
<script type="text/javascript" src="dojo/dojo.js"></script>
<script>
require(['dojo/on', "dojo/touch","dojox/mobile/parser",'dojo/domReady!', 'dojox/mobile/View', 'dojox/mobile/RoundRectList', 'dojox/mobile/ListItem'],function(on, touch, parser){
	parser.parse();
	on(document.getElementById('clickBtn'),touch.press, function(){	alert('CLICKED'); });
});
</script>
</body>

the event bound with dojo.on always works, however the link on dojox/mobile/ListItem does not work with the current code

comment:23 Changed 5 years ago by Adrian Vasiliu

Thanks @ZiggyQubert? for the clarification and code, but I don't reproduce. This is how I tested:

  1. Copied your code into a test.html placed in the root dir of a freshly updated set of checkouts of the 1.9 branch of the dojo, dijit, dojox, and util git repos.
  2. Loaded it in Safari / iOS 8 beta 5 on a physical iPhone 5S and also simulators from Xcode 6.0 (6A279r) (tried on iPhone 5S and iPad Air).
  3. Touched the ListItem? "Go to CNN - dosent work with existing dojo/on code".

==> The cnn site loads as expected (and the console of Safari's webinspector stays clean of any error).

I don't know what could make it work differently on your side. Did you consistently use the latest state of the 1.9 branch on all dojo repos (dojo, dijit, dojox)?

Last edited 5 years ago by Adrian Vasiliu (previous) (diff)

comment:24 Changed 5 years ago by Sherbrooke Andrews

So it looks like its something to do with the built version, I did a build with the dojo layer as follows

    layers: {
      "dojo/dojo" : {
    	  include: [ "dojo/on" ],
			exclude: [],
    	}
    }

the uncompressed version of the file works fine, the compressed version of the file dosen't work, not compleatly sure why. I'll do some more testing tomorrow, and can post the full profile I'm using if you need.

comment:25 Changed 5 years ago by Adrian Vasiliu

Okay, so it's likely the issue would go away once the build issue is solved. I'm not a build expert myself, but I guess posting the full profile may help.

comment:26 Changed 5 years ago by Adrian Vasiliu

Also, you can check the dojo/on part of the uncompressed version of the layer (dojo.js.uncompressed.js).

comment:27 Changed 5 years ago by Sherbrooke Andrews

Ok, so with a bit more digging I've found the exact cause of the issue, dojo/on is declaring "use strict"

define(["./has!dom-addeventlistener?:./aspect", "./_base/kernel", "./sniff"], function(aspect, dojo, has){

	"use strict";
	if( 1 ){ // check to make sure we are in a browser, this module should work anywhere

which the closure compiler strips out when it minifies the files.

with use strict on, iOS 8 will throw an error when you attempt to modify a read only property, when its off it wont throw an error.

As far as I know there is no way to pass closure an extra parameter to keep the "use strict" directive, closure supports this (with --compiler_flags="--language_in=ECMASCRIPT5") but the dojo build system doesn't provide a mechanism to pass the flags to closure without editing the build scripts).

In any case I'd say that the code should support the default build configuration (where use strict is removed), and it probably easier to update this code rather than the build system.

comment:28 Changed 5 years ago by Adrian Vasiliu

Got it, thanks a lot. Again, I'm not a build expert, but I can tell that, depending on the build system choices/configuration, there are means to avoid that the "use strict" gets stripped. That said, it's clearly better to avoid the need to take care of it at build level. I'll probably rework it in this sense.

Last edited 5 years ago by Adrian Vasiliu (previous) (diff)

comment:29 Changed 5 years ago by Adrian Vasiliu

Submitted this pull request for fixing this issue: https://github.com/dojo/dojo/pull/110 (this is against dojo's master; in a second step I'll backport it).

@ZiggyQubert?: thanks again for your input on it.

Last edited 5 years ago by Adrian Vasiliu (previous) (diff)

comment:30 Changed 5 years ago by Adrian Vasiliu

The dojo/on fix for iOS 8 (beta) is now merged into the master, 1.10, 1.9, and 1.8 branches of the dojo repository.

comment:31 Changed 5 years ago by Adrian Vasiliu <vasiliu@…>

In 6012aedfea45220b6e84a7544f45d39b9fdad4f6/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:32 Changed 5 years ago by Adrian Vasiliu <vasiliu@…>

In 37e50db764591c4982a20246d3e7537ffdceac9a/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:33 Changed 5 years ago by Colin Snover

Milestone: 1.10.11.10.2

1.10.1 has been released, retargeting all open tickets to next milestone.

comment:34 Changed 5 years ago by bill

pruzand - Shouldn't this ticket be marked as fixed in 1.10.1? Or is there more work left?

comment:35 Changed 5 years ago by Patrick Ruzand

Milestone: 1.10.21.10.1
Resolution: fixed
Status: assignedclosed

comment:36 Changed 5 years ago by Adrian Vasiliu

The dojo/on fix for iOS 8 is now also backported to 1.7 (in https://github.com/dojo/dojo/commit/7b156f9ca999534709c17a850278184e4bcf153d). So it's now merged into master and in the 1.10, 1.9, 1.8, and 1.7 branches of the dojo repository.

comment:37 Changed 5 years ago by bill

Milestone: 1.10.11.7.7

Thanks, IIUC this will be in 1.7.7, 1.8.8, 1.9.5, and is already in 1.10.1.

Marking milestone as "earliest" version fixed.

In the future please use "cherry-pick -x" to do backports so it notes the original commit.

comment:38 Changed 5 years ago by Adrian Vasiliu

In the future please use "cherry-pick -x" to do backports so it notes the original commit.

Just a bit of explanation. This is what I usually do, and this is what I first tried here too, but it failed because of a merge conflict due to a another commit with a tiny change in the same code (https://github.com/dojo/dojo/commit/ca9a818ac07153a92f7548b2acd2ea7bd05af602), done in master but not in older branches. I'm not sure cherry-picking is compatible with manually resolving merge conflicts (is it? I would think it isn't.). Additionally, there are two, not one original commit in master, 1.10 and 1.9: an initial fix, plus an additional fix. Later, when the backport to 1.8 and 1.7 was decided, I merged these into a single commit for the older branches.

comment:39 Changed 5 years ago by bill

Thanks.

You can use cherry-pick when there's a merge conflict: just resolve the conflict, call "git add" on the conflicting files, and then "git commit".

But really, I just meant to somehow note which commit you are backporting in the check-in comment. Just for the future, in case we need that information for some reason.

comment:40 Changed 5 years ago by Adrian Vasiliu

Okay. I thought the reference to the Trac ticket in the commit message is enough, because the ticket references the other commits, but you're right, it could be more clearly indicated. So thanks.

comment:41 Changed 5 years ago by Adrian Vasiliu

@bill

IIUC this will be in 1.7.7, 1.8.8, 1.9.5, and is already in 1.10.1.

AFAIK, it is more exactly as follows:

The dojo/on fix is in 1.8.7, 1.9.4, 1.10.1 (all already shipped), and it will be in 1.7.7 (not yet shipped).

Last edited 5 years ago by Adrian Vasiliu (previous) (diff)

comment:42 Changed 5 years ago by ysazak

my application works fine on IOS8 after I upgraded to 1.8.8. But now it doesnt work on Android. No touch event fired.

comment:43 Changed 5 years ago by cjolif

ysazak, see our request for details here: https://bugs.dojotoolkit.org/ticket/18322#comment:2

comment:44 Changed 5 years ago by Adrian Vasiliu

ysazak, this is unrelated with the changes for iOS 8 (#18168). See https://bugs.dojotoolkit.org/ticket/18322#comment:4.

comment:45 Changed 5 years ago by bill

#18380 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.