Skip to content
This repository has been archived by the owner on Sep 4, 2019. It is now read-only.

swipemenu.js issues in Tablet OS 2.1 BETA #31

Open
astanley opened this issue Jun 6, 2012 · 11 comments
Open

swipemenu.js issues in Tablet OS 2.1 BETA #31

astanley opened this issue Jun 6, 2012 · 11 comments
Labels

Comments

@astanley
Copy link

astanley commented Jun 6, 2012

Hearing reports from developers that swipemenu.js is broken when run from within the Tablet OS 2.1 BETA.

I've just updated my device to the 2.1 BETA and re-ran the sample app included in the swipemenu.js project and did not see any problems.

If you are seeing issues, can you please report them here?

@nunodonato
Copy link

I can confirm this issue. Had swipemenu.js working fine and after the 2.1 Update it stopped working.
The inspector doesn't show any errors.
Any suggestions to help finding out the cause?

@astanley
Copy link
Author

astanley commented Jun 6, 2012

@nunodonato
Copy link

I tracked this down to createSwipeMenu function, more specifically this line
rightButtons.style = style;
where a try..catch block throws out this message "Attempted to assign to readonly property"

Guess from here it's out of my capacity to debug :)

@nunodonato
Copy link

I just removed the "use strict" from swipemenu.js and everything works fine.
Hopefully this will give some hints on where the problem is in 2.1. But its strange why Adam doesn't get the same behaviour

@glasspear
Copy link

I can't give you too much to go on. But I can replicate the issue.

Just did a deploy with (sign + debug) and onSwipeDown gives no errors (no function calls either), but menu does not appear.

If you hit the 'simulate bezel swipe' button the JS fires but the menu doesn't appear (console.log info):
showMenuBar complete (swipemenu.js:46)
simulateSwipeEvent complete ((swipemenu.js:175)

PB OS: 2.1.0.560

(p.s. as a test I installed the bbUI sample app and onSwipeDown works in that app)

@nunodonato
Copy link

There is no problem with the swipedown event.
Like I said above, the problem is in that particular line while using "use strict". It throws an exception, the code breaks and nothing else gets loaded.

@glasspear
Copy link

Yup I know, just trying to give Adam all the details I can :)

Adam - nunodonato has it the tip of the iceberg on this one.

It isn't just that one re-assignment (rightButtons.style = style) I removed a lot of the re-assignments and it keeps finding me more to remove. It appears that any time you assign style = obj.style then work with it and assign it back obj.style = style it is choking because it thinks obj.style is readonly.

The reason removing "use strict" 'fixes' it is because it is just failing silently without it. This is also why bbUI doesn't appear to be affected as it doesn't have 'use strict'.

I just removed all the re-assignments and the swipe menu works fine again with "use strict" still in place. I know this is not an ideal solution as this was originally done to reduce the screen re-writes.

@astanley
Copy link
Author

Feedback from our browser folks:

"style" is clearly defined as "read-only" here: http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Element.idl
line 64: readonly attribute CSSStyleDeclaration style;

They think this is an old bug that was finally fixed in WebKit.
So this issue should also be happening in the latest Chrome / Safari as its a WebKit change that was picked up in Tablet OS 2.1 BETA.

Bit of a surprise, but there it is. We will just need to identify the right way to do this and make a change.

@glasspear
Copy link

Is the purpose of this in and out of variables trick is to reduce redraws on the screen?

My (maybe flawed) thought is this: if the elements have just been created and not yet attached to the DOM does it really cause a redraw? (my knowledge of this sort of manipulation is still budding) Could this step just be skipped on items that aren't attached to the DOM yet?

@astanley
Copy link
Author

Yes, the purpose of this technique is to reduce page repaints. However for newly create elements, this is not an issue so we can work around it for this. However, for live DOM content, I would like to explore what is now the most efficient way of making style changes like these.

So swipemenu.js will have to be updated to adjust for the latest changes to WebKit. Style is a readonly property now.

@astanley
Copy link
Author

I'm working on fixing this today. Hoping to submit a pull request this afternoon.

astanley pushed a commit to astanley/WebWorks-Samples that referenced this issue Jun 19, 2012
astanley pushed a commit that referenced this issue Jun 22, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants