-
Notifications
You must be signed in to change notification settings - Fork 949
Test failure on iOS 8: child and adjacent -> p#firstp + p #290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
This is blocking PR #289. |
Have you investigated manually? |
I didn't have time to look at it more closely yet, I'll try soon. This is reproducible both in emulators and on real devices FWIW. |
It is probably the bug https://bugs.webkit.org/show_bug.cgi?id=136851 fixed in http://trac.webkit.org/changeset/173688 Compound selectors fail to use the right origin in some cases. |
@BenjaminPoulain Thanks for the links. This really should be fixed in a patch release, it's a serious bug. @gibson042 I've confirmed on a raw example, withouth jQuery. |
I'd like to wait and see on this one, then... code to fix a bug present only in iOS 8.0.2–8.0.4 (for example) doesn't thrill me. |
It's unlikely there will be any further 8.0.x releases; 8.1.0 might be released soon, though. Let's see. Maybe they'll also manage to fix the user agent in 8.1 simulator, that makes it impossible for us to test iOS8 in jQuery. |
What's the bug with the simulator's user-agent? Did you file a report on bugzilla and/or radar? |
@BenjaminPoulain I filed a bug report via the Feedback Assistant from Yosemite public beta. See tobie/ua-parser#450 for more info. |
The issue is not fixed in iOS 8.1. The user agent of a real iOS is correct; the one from iOS 8.1 simulator in the final XCode 6.1 is still wrong. |
I've reported the issue once again via https://bugreport.apple.com |
There is no need, the issue is fixed in WebKit. The reason the patch does not make it to a minor update is the lack of user impact. The patch should come eventually with the next branching from WebKit. |
It would be nice if Apple was friendlier towards developers. |
Sorry for the misunderstanding, I did not mean to split "users" and "developers" as different categories. WebKit is a framework, its "users" are people making website: developers. What I meant is nobody reported this as an important problem affecting a website. The bug was fixed in WebKit trunk. This area is better tested now to make sure such problem never happen again. It will eventually make it to the stable WebKit. |
I understand it; however, this problem makes it impossible for us to test |
@gibson042 Since Apple doesn't seem to consider this bug as important enough to backport the fix from current WebKit, should we just blacklist the test on iOS8? Or do you think we should workaround it? |
Things that don't work:
Things that do work:
It seems that Update: iOS 8.1.1 went out, this bug is not fixed there. |
I'm still 👎 on landing a fix like this. If Apple doesn't think it's serious, who are we to argue? |
This problem also present in desktop Safari 8.0, that's worrisome since this is a major version whereas we usually wait pretty good amount of time for Apple to release even a minor release for desktop. This also prevents us to add desktop and mobile Safari to Sizzle browser launchers and we constantly see this problem with integration tests in Core - http://swarm.jquery.org/project/jquery |
Well, for Apple a minor update is just the current Safari with the engine from the next major version so it's not surprising we wait for those (as they release major updates once a year). Also, because this is effectively a new browser version we will still need to test on Safari 8.0.x even when 8.1.0 is released. So unless Apple fixes it in a patch release which seems unlikely we either have to workaround it in Sizzle or blacklist the test for Safari 8. The latter seems ugly; we usually don't treat "modern" browsers in this way if we can fix a specific bug. |
If it were Chrome, I'd be less inclined to land the fix. Since Apple has a much slower release cycle, I'm leaning towards landing. |
That said, we can land the fix in the same version that removes legacy code. It won't be a lot, but it will likely make room for the fix. |
I assume it may take some time? We should then blacklist this test in Safari 8/iOS 8 for the time before the fix lands so that we can actually test on these browsers and care about other failures. |
@mzgol We can go ahead and land now. We just wouldn't release the next version of Sizzle until the other changes were made. |
@timmywil Landing in Sizzle won't prevent Core from getting the build failure until Core can get the updated Sizzle: |
Right, I was only thinking of the Sizzle repo. For Core, it might be a good case for QUnit.skip(). |
Ok then, if most of the team thinks it should be fixed, let's fix it. How about if I also do a blog post, since this is exactly the kind of thing that jQuery can protect you from vs using bare |
Per our meeting today, we're putting out an urgent patch for 1.11.2/2.1.2 to address this, since 3.0 is a month or more away and a higher risk upgrade. @BenjaminPoulain if you have any idea about when Safari will pull in the WebKit fix, now would be a good time to let us know. |
Or if, as I suspect, you're not allowed to share such info even if you know it, it'd be great if you could raise it internally at Apple that we're planning a release just because of this bug. It would help if we didn't have to do it. |
I am sorry, I cannot really say more. |
@BenjaminPoulain That says a lot doesn't it? 😼 The link to the WebKit bug was very helpful so we could understand the scope of the bug. Please do comment when you can, we appreciate it. |
I'm not sure if this should be in, but I guess it could save people a lot of headache knowing about this bug. jquery/sizzle#290,
Letting people know about this bug could save some serious headaches. jquery/sizzle#290
We have one test failure on iOS8:
The text was updated successfully, but these errors were encountered: