Skip to content
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

Add a local jQuery fallback. #16888

Merged
merged 1 commit into from
Jul 26, 2015
Merged

Add a local jQuery fallback. #16888

merged 1 commit into from
Jul 26, 2015

Conversation

XhmikosR
Copy link
Member

Shouldn't really happen, but China for example has blocked Google so this should help.

Fixes #16886.

/CC @cvrebert

PS. I skipped template.html on purpose.

@XhmikosR XhmikosR added the docs label Jul 25, 2015
@XhmikosR XhmikosR added this to the v3.3.6 milestone Jul 25, 2015
@@ -169,6 +169,7 @@ <h2 class="blog-post-title">New feature</h2>
================================================== -->
<!-- Placed at the end of the document so the pages load faster -->
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.3/jquery.min.js"></script>
<script>window.jQuery || document.write('<script src="../../assets/js/vendor/jquery-1.11.3.min.js"><\/script>')</script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps omit the version from the filename for easier upgrading

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it, but I went with this. I don't have any strong opinion since GitHub doesn't really cache the CSS/JS files for a long time, so it should be OK if we use the same filename.

@cvrebert
Copy link
Collaborator

Please find another reviewer for this.

@XhmikosR
Copy link
Member Author

Not sure what you mean. The patch is pretty straightforward, isn't it?

@cvrebert
Copy link
Collaborator

Yes, but I DGAF about the underlying issue, so I'm not a good person to review this.

@XhmikosR
Copy link
Member Author

I know for sure GitHub pages set the cache to like 10 min for CSS and JS files so it's OK (for now at least).

Generally, it's always preferable to use a distinct filename for caching issues.

@cvrebert
Copy link
Collaborator

I know for sure GitHub pages set the cache to like 10 min for CSS and JS files so it's OK (for now at least).

Right, it's just that having to modify 15 files to upgrade the jQuery version in the future seems potentially annoying.

@XhmikosR
Copy link
Member Author

True but it'd just be a search and replace.

I'll make a new patch tomorrow with this change.
On Jul 26, 2015 12:52 AM, "Chris Rebert" [email protected] wrote:

I know for sure GitHub pages set the cache to like 10 min for CSS and JS
files so it's OK (for now at least).

Right, it's just that having to modify 15 files to upgrade the jQuery
version in the future seems potentially annoying.


Reply to this email directly or view it on GitHub
#16888 (comment).

Shouldn't really happen, but China for example has blocked Google so this should help.
@XhmikosR
Copy link
Member Author

Used jquery.min.js as the filename.

XhmikosR added a commit that referenced this pull request Jul 26, 2015
@XhmikosR XhmikosR merged commit 9433924 into master Jul 26, 2015
@XhmikosR XhmikosR deleted the jquery-local-fallback branch July 26, 2015 08:02
@mdo mdo mentioned this pull request Jul 26, 2015
@cvrebert
Copy link
Collaborator

cvrebert commented Sep 8, 2015

@XhmikosR Has this been ported to v4's docs?

@XhmikosR
Copy link
Member Author

XhmikosR commented Sep 8, 2015

I think it has, yes. And I added the missing fallback in 85ab831.

@cvrebert
Copy link
Collaborator

cvrebert commented Sep 8, 2015

Superb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants