Skip to content

Deferred script loading #3656

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.

Sign up for GitHub

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

Jump to bottom
Closed
michaelletzgus wants to merge 2 commits into from
Closed

Deferred script loading #3656

michaelletzgus wants to merge 2 commits into from

Conversation

@michaelletzgus
Copy link
Contributor

@michaelletzgus michaelletzgus commented Feb 28, 2017

Discussed here: #2272

@mention-bot
Copy link

mention-bot commented Feb 28, 2017

@michaelletzgus, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nickvergessen, @LukasReschke and @ChristophWurst to be potential reviewers.

@michaelletzgus michaelletzgus mentioned this pull request Feb 28, 2017
Closed
@benediktg
Copy link
Member

benediktg commented Feb 28, 2017

I would recommend to squash these commits into a single one. 😉

LukasReschke
LukasReschke requested changes Feb 28, 2017
View reviewed changes
Copy link
Member

@LukasReschke LukasReschke left a comment

Choose a reason for hiding this comment

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

Before we merge that I'd love to have some official feedback from Mozilla here: https://bugzilla.mozilla.org/show_bug.cgi?id=1342781

@LukasReschke
Copy link
Member

LukasReschke commented Feb 28, 2017

@jancborchardt @jospoortvliet Knowing any Mozilla people? 😉

@michaelletzgus
Copy link
Contributor Author

michaelletzgus commented Mar 1, 2017

@benediktg
Can I squash the 4 commits or do I need write access to this repository?
Or should I commit all 4 files together? I did the changes online, without a local repository. My local test-repository is not working yet, there is some trouble with nginx and a repository as root...

@benediktg
Copy link
Member

benediktg commented Mar 1, 2017 •
edited by nickvergessen
Loading

(slightly off-topic)

Can I squash the 4 commits or do I need write access to this repository?

You can squash the latest 4 commits in the current branch by doing

git rebase -i HEAD~4
# replace the lower 3 occurences of 'pick' by 's'
git commit --amend -s
git push --force-with-lease

in the branch of you own fork. Otherwise also the project maintainers could squash the commits at merging.

@michaelletzgus michaelletzgus force-pushed the deferred-script-loading branch from eafb0f2 to f518eeb Compare March 2, 2017 08:58
@michaelletzgus
Copy link
Contributor Author

michaelletzgus commented Mar 2, 2017

Done

@michaelletzgus
Deferred script loading, scripts from AddOns
fcc2fc8 
Add the 'defer' attribute to external script requests.
Warning 1: This is only OK when the whole page is using 'defer', otherwise script execution order might be wrong!
Warning 2: IE<=9 does not support 'defer' properly!

Deferred script loading

* Added 'defer' to jsfile loading
* Moved inline js loading  down for not interrupting deferred css and js loading
@michaelletzgus michaelletzgus force-pushed the deferred-script-loading branch from f518eeb to fcc2fc8 Compare March 2, 2017 09:01
@michaelletzgus
Copy link
Contributor Author

michaelletzgus commented Mar 3, 2017

Please see PR #3696

@michaelletzgus michaelletzgus mentioned this pull request Mar 3, 2017
Closed
@michaelletzgus michaelletzgus deleted the deferred-script-loading branch March 20, 2017 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LukasReschke LukasReschke LukasReschke requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@michaelletzgus @mention-bot @benediktg @LukasReschke