Moodle

session in tinymce setup code prevents running of javascript on page before it is fully loaded

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: HTML Editor
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

The problem is that the included tinymce setup script needed access to session, but it can not obtain it until the original page loading finishes.
Solution is to pass everything through page params and do not use session or else do the tinymce setup on each page.

Issue Links

Activity

Hide
Petr Škoda (skodak) added a comment -

tinymce integration does not use session anymore, there is still problem with CSS (editor changes text size outside of it)
could you please review it and fix the css issue? thanks

Show
Petr Škoda (skodak) added a comment - tinymce integration does not use session anymore, there is still problem with CSS (editor changes text size outside of it) could you please review it and fix the css issue? thanks
Hide
Petr Škoda (skodak) added a comment -

hmm, looking at the performance, do we really have to load the setup code for editor on each page even when not used there?

Show
Petr Škoda (skodak) added a comment - hmm, looking at the performance, do we really have to load the setup code for editor on each page even when not used there?
Hide
Mathieu Petit-Clair added a comment -

Well, as we don't know (in the header) if we will need the editor later on, there's two choices

1) load it on every page, in the header, which is clean and simple
2) load it when necessary, in the middle of the html code, and add some checks to make sure it's only loaded once (not too messy, as long as people use print_textarea function)

I guess I'll go back to the "on demand in the middle of the page" thing, as we have to do that for the "file picker" anyway, though I find this ugly.

Show
Mathieu Petit-Clair added a comment - Well, as we don't know (in the header) if we will need the editor later on, there's two choices 1) load it on every page, in the header, which is clean and simple 2) load it when necessary, in the middle of the html code, and add some checks to make sure it's only loaded once (not too messy, as long as people use print_textarea function) I guess I'll go back to the "on demand in the middle of the page" thing, as we have to do that for the "file picker" anyway, though I find this ugly.
Hide
Mathieu Petit-Clair added a comment -

Petr, thinking this over again after some caffeine... The TinyMCE code is linked on every page, but it's not dynamically generated (it's pure JS, contrary to what we were doing with htmlarea) so it should be cached by the browser. And it's fully loaded only when there is an editor to load, I don't think it's a performance problem. There is a cost, but it's very small (afaics, one DOM call to see if the proper CSS classes exists) on page where the editor is not necessary.

It's certainly not worse than adding the equivalent code to core and it makes a cleaner output.

Show
Mathieu Petit-Clair added a comment - Petr, thinking this over again after some caffeine... The TinyMCE code is linked on every page, but it's not dynamically generated (it's pure JS, contrary to what we were doing with htmlarea) so it should be cached by the browser. And it's fully loaded only when there is an editor to load, I don't think it's a performance problem. There is a cost, but it's very small (afaics, one DOM call to see if the proper CSS classes exists) on page where the editor is not necessary. It's certainly not worse than adding the equivalent code to core and it makes a cleaner output.
Hide
Mathieu Petit-Clair added a comment -

The latest commit (see MDL-15688) should fix the session issue, using NO_MOODLE_COOKIES in all relevant scripts.

As per Martin's suggestion, I'll set the script up so that caching is done as often as possible.

Show
Mathieu Petit-Clair added a comment - The latest commit (see MDL-15688) should fix the session issue, using NO_MOODLE_COOKIES in all relevant scripts. As per Martin's suggestion, I'll set the script up so that caching is done as often as possible.
Hide
Petr Škoda (skodak) added a comment -

thanks

Show
Petr Škoda (skodak) added a comment - thanks

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: