scottjehl / iOS-Orientationchange-Fix

A fix for the iOS orientationchange zoom bug.
770 stars 115 forks source link

Quarantine regex is too restrictive #18

Closed aaronbarker closed 12 years ago

aaronbarker commented 12 years ago

The quarantine regex only allowed for OS 5_1, but not 5 or 5_1_1 (the current version). My regex-fu may not be perfect, but I believe the modification should allow for all 3 of the above.

scottjehl commented 12 years ago

Thanks, but this should have landed already in master (as of yesterday).

Is it not working properly currently for you?

On Jun 21, 2012, at 7:21 PM, Aaron Barker wrote:

The quarantine regex only allowed for OS 5_1, but not 5 or 5_1_1 (the current version). My regex-fu may not be perfect, but I believe the modification should allow for all 3 of the above.

You can merge this Pull Request by running:

git pull https://github.com/aaronbarker/iOS-Orientationchange-Fix master

Or you can view, comment on it, or merge it online at:

https://github.com/scottjehl/iOS-Orientationchange-Fix/pull/18

-- Commit Summary --

  • The quarantine regex only allowed for OS 5_1, but not 5 or 5_1_1 (the current version). My regex-fu may not be perfect, but I believe the modification should allow for all 3 of the above.

-- File Changes --

M ios-orientationchange-fix.js (2)

-- Patch Links --

https://github.com/scottjehl/iOS-Orientationchange-Fix/pull/18.patch https://github.com/scottjehl/iOS-Orientationchange-Fix/pull/18.diff


Reply to this email directly or view it on GitHub: https://github.com/scottjehl/iOS-Orientationchange-Fix/pull/18

aaronbarker commented 12 years ago

It is not working for me. I'm on 5_11 but the regex only checks for [1-5]\d (no second _1) so fails.

Scotchester commented 12 years ago

Looks like it's landed in the .js but has not been updated in the minified code in the readme.

scottjehl commented 12 years ago

Looks fine to me? https://github.com/scottjehl/iOS-Orientationchange-Fix#readme

Scotchester commented 12 years ago

Weird... says you made that change 34 minutes ago, but I'm pretty sure I saw it more recently without that change... A caching issue, perhaps. I definitely copied it less than an hour ago with the old regex.

aaronbarker commented 12 years ago

Cool thanks for the fix.

If I might ask, for my own education, what makes your regex better than mine? Yours: [1-5][0-9] Mine: [1-5][_\d]

This was my first patch submission, so just trying to be educated if it is easier to make a change as the author than accept a pull request, or if the regex is better in some way.

Not trying to be flippant or challenging, just trying to learn :)

scottjehl commented 12 years ago

Ah, actually yours looks better I think. I was merely pointing out that when this pull came in, the fix was already in master. If you want to resend this as a commit on top of master, I'd be happy to pull it in.

Thanks!