Wilson Page [:wilsonpage] Assignee
	Description • 11 years ago

      No description provided.

	Wilson Page [:wilsonpage] Assignee
	Comment 3 • 10 years ago

yor, kgrandon, gmarty: You should check out the changes I made to `ComponentUtils.style()`. Seems to fix my @import woes.

	Yan Or
	Comment 4 • 10 years ago

(In reply to Wilson Page [:wilsonpage] from comment #3)
> yor, kgrandon, gmarty: You should check out the changes I made to
> `ComponentUtils.style()`. Seems to fix my @import woes.

AWESOME!  It's working!  Please, please, please land this fix today.

	Yan Or
	Comment 5 • 10 years ago

Comment on attachment 8430768 [details]
pull-request (master)

Looks good.  Assuming Travis passes.

	Yan Or
	Comment 6 • 10 years ago

Hey Kevin, if this looks good to you, can you land this today?  Thanks!

	Kevin Grandon :kgrandon
	Comment 7 • 10 years ago

Comment on attachment 8430768 [details]
pull-request (master)

Something weird is going here on travis and I'm not sure what. I've restarted the job 3 times now but it keeps timing out. (Some other jobs are, but not this much).

Can you try rebasing against master and force pushing to re-trigger travis?

Thanks!

	Yan Or
	Comment 8 • 10 years ago

It keeps timing out here:

  ․․․․․Xlib:  extension "RANDR" missing on display ":99.0".

I suggest if the review looks good that we move on and land this.  A lot of other tickets depend on the @import fix.

Thanks.

	Wilson Page [:wilsonpage] Assignee
	Comment 9 • 10 years ago

I seems to have passed on Gaia-Try [1], going to land this to free up Yan. 

[1] https://hg.mozilla.org/integration/gaia-try/rev/312800e37e84

	Wilson Page [:wilsonpage] Assignee
	Comment 10 • 10 years ago

Landed on 'master' https://github.com/mozilla-b2g/gaia/commit/392123a9c53fd9a6ccbb1c81908c3176c0647222

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 10 years ago

Revert for Gaia unit test failures.
Master: https://github.com/mozilla-b2g/gaia/commit/97de0a1b593ff97a26bcbac4e006179ec882466b

https://tbpl.mozilla.org/php/getParsedLog.php?id=40746374&tree=B2g-Inbound

	Kevin Grandon :kgrandon
	Comment 12 • 10 years ago

Come on guys, I told you to wait for a green travis. Sometimes it's ok to not wait. When someone asks you to wait for it, please wait.

	Yan Or
	Comment 13 • 10 years ago

Sorry, I asked Wilson to land it.  Do you think the timeouts are related to this patch?

	Kevin Grandon :kgrandon
	Comment 14 • 10 years ago

It's possible, but I don't know off the top of my head. We should run the tests locally first. If there's some reason we can't reproduce we should try a reduced change on travis, or running just the camera tests on travis.

	Yan Or
	Comment 15 • 10 years ago

Kevin, you are right.  I did a PR with just the component_utils.js changes and it's failing the test as well:

https://travis-ci.org/mozilla-b2g/gaia/builds/26427721

But another PR for an unrelated gaia-subheader change is passing:

https://travis-ci.org/mozilla-b2g/gaia/builds/26428551

	Kevin Grandon :kgrandon
	Comment 16 • 10 years ago

It could be that changing component utils to use onload is not working. We may need to spin the event loop inside the test using sinon fake timers after the onload. Let me know if you have trouble and ni? me - I can write a small patch for you.

	Kevin Grandon :kgrandon
	Comment 17 • 10 years ago

Turns out we're actually getting a platform crash in the set visibility call for some reason... 

https://crash-stats.mozilla.com/report/index/09858a26-ab90-4e5c-92f7-aabd02140531

Appears to be something in the test. While normally we could disable some tests if absolutely necessary, I'm scared to do that when we encounter a crash. Let me look at options and see if I can find a workaround.

	Yan Or
	Comment 18 • 10 years ago

Something is wrong with Travis.  Now the same PR with just component_utils.js is failing the gaia unit tests.  It passed before.

	Kevin Grandon :kgrandon
	Comment 19 • 10 years ago

I think the attempted component utils fix may be rearing up a nasty platform bug. It may be possible to workaround, or we may have to wait for the @import fix.

	Yan Or
	Comment 20 • 10 years ago

I commented out the 'visibility' calls and got the unit tests to work.  Can we go with this until we have the permanent platform fix?

	Kevin Grandon :kgrandon
	Comment 21 • 10 years ago

Comment on attachment 8432230 [details]
component_utils.js fix

We should not have dead code, so R- for that for now. Does changing to onload fix the problem that we were having? I'm still not really seeing any problems with setTimeout, but if onload works, that's fine too.

	Yan Or
	Comment 22 • 10 years ago

Comment on attachment 8432230 [details]
component_utils.js fix

Yes, in some cases, the rendering was still not right with setTimeout().  I've made the changes you suggested.

Wilson, can you remove component_utils.js from your patch?

	Kevin Grandon :kgrandon
	Comment 23 • 10 years ago

Comment on attachment 8432230 [details]
component_utils.js fix

Looks fine to me.

	Wilson Page [:wilsonpage] Assignee
	Comment 25 • 10 years ago

Apologies for landing this broken stuff! At the time Travis was in complete meltdown taking >8hrs to run a build. I took a look at the gaia-try output and which seemed to be green. I guess someone needs to clarify with me what gaia-try is and how to read it properly. I'm guessing Kevin knows.

The `style.visibility` stuff was required to prevent 'FOUC' before the component style-sheet had loaded. Where were you seeing the crash happen? Was it in the unit-tests or on in app on device? If the component utils stuff doesn't fix this then I can't land this Camera patch, as you can see a flicker of unstyled header when first opening the preview-gallery.

Sorry again chaps; let's all sync up on IRC when you're online.

	Kevin Grandon :kgrandon
	Comment 26 • 10 years ago

It was crashing during unit tests, and likely because travis runs in firefox and tbpl runs in b2g desktop. The unfortunate meltdown was happening because people were testing several hours worth of automated tests =/

I'm a bit slammed this week, but might be able to look at other options. We can explore off-screen positioning, or some other style attribute to avoid FOUC perhaps.

	Wilson Page [:wilsonpage] Assignee
	Comment 27 • 10 years ago

yor: Identified and fixed the crashing issue. Firefox seemed to be crashing with a combination of the `new GaiaHeader()` type instantiation and manipulating the element's `style` property on creation. I have pushed an updated patch that re-instates the FOUC fix and fixes the unit-tests.

	Yan Or
	Comment 28 • 10 years ago

Nice work, Wilson!  Unit tests pass for me locally and the fix works fine in app.

I see Travis has some failures though.

	Wilson Page [:wilsonpage] Assignee
	Comment 29 • 10 years ago

Travis is green now, not sure if we're allowed to land this for v2.0 now are we :-/

	Yan Or
	Comment 30 • 10 years ago

Can you break the patch into 2 parts, one for Camera and the other for component_utils?  We can then land just the latter and continue porting apps.

	Diego Marcos [:dmarcos]
	Comment 31 • 10 years ago

Comment on attachment 8432384 [details]
pull-request (master)

This looks great to me.

	Wilson Page [:wilsonpage] Assignee
	Comment 32 • 10 years ago

Landed on 'master' 

https://github.com/mozilla-b2g/gaia/commit/32a14364f26bf96996319717351fcb1e7fb48605

	Safwan Rahman (:safwan)
	Comment 33 • 10 years ago

It seems that, this patch raised bug 1079953. 
Can you please take a look?

	Wilson Page [:wilsonpage] Assignee
	Comment 34 • 10 years ago

This looks like a window management bug to me. I've NI'd someone relevant on the other bug.