Skip to content

Don't include box2d JS for IE8 and below (Bug 803367)#427

Closed
sgarrity wants to merge 3 commits into
mozilla:masterfrom
sgarrity:bug-803367-apps-ie-js
Closed

Don't include box2d JS for IE8 and below (Bug 803367)#427
sgarrity wants to merge 3 commits into
mozilla:masterfrom
sgarrity:bug-803367-apps-ie-js

Conversation

@sgarrity
Copy link
Copy Markdown
Contributor

Replacing messy PR #426

@rik
Copy link
Copy Markdown
Contributor

rik commented Oct 23, 2012

Merged in 1532f1c and 4e6dcfd

@rik rik closed this Oct 23, 2012
@rik
Copy link
Copy Markdown
Contributor

rik commented Oct 23, 2012

Oops, wrong pull request, nothing merged yet.

@rik rik reopened this Oct 23, 2012
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be merged into one file.

@rik
Copy link
Copy Markdown
Contributor

rik commented Oct 23, 2012

So this does way more than just not executing box2d. I feel like some CSS changes are here to fix the layout on old IEs but there's also a bunch of deletion in #marketplace-partners. Could you explicit those?

@sgarrity
Copy link
Copy Markdown
Contributor Author

The #marketplace-partners CSS was for the old partners page, which has been removed. Some other minor changes have been made for IE7/8 layout improvements.

The box2d js is now combined/minified.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be changed to if (!$.fn.jQb2) { return; } to avoid an extra level of indentation.

@rik
Copy link
Copy Markdown
Contributor

rik commented Oct 24, 2012

My bad, as @sgarrity pointed out, this function does more than just jQb2 stuff so early return are not ok.

I'll merge this.

@rik
Copy link
Copy Markdown
Contributor

rik commented Oct 24, 2012

This is merge in f169584.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants