Closed Bug 797703 Opened 12 years ago Closed 12 years ago

moodle serves plain textarea instead of rich text editor

Categories

(Tech Evangelism Graveyard :: English US, defect)

Product:
Tech Evangelism Graveyard
Component:
English US
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox17-, firefox18-)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox17 - ---
firefox18 - ---

People

(Reporter: Hughman, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 obsolete files) 

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20120926042010

Steps to reproduce:

Since I upgraded to Aurora 17 a few weeks ago I have noticed that a number of sites do not have their rich text editors loading. These sites include Moodle and DotNetNuke. So this causes a large user base to be at disadvantage.

I have compared to other browsers and even back to Beta 16 at one point and all was working fine. Tried a new profile in Aurora 17 and the problem still existed.

Example site to use can be http://demo.moodle.net/user/edit.php?id=5&course=1 where after using demo login as student, the "description" field should be rich text.


Actual results:

The textarea which should have progressively enhanced stays as a textarea.


Expected results:

It should have shown all the rich text buttons and things.
On http://demo.moodle.net/user/edit.php?id=5&course=1, UA spoofing Gecko/<date> helps .
Ex. user_pref("general.useragent.override", "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20100101 Firefox/17.0");

So this may be Tech Evangelism bug.
Blocks: 588909
Status: UNCONFIRMED → NEW
tracking-firefox17: --- → ?
tracking-firefox18: --- → ?
Ever confirmed: true
Keywords: regression
Assignee: nobody → english-us
Component: Untriaged → English US
Product: Firefox → Tech Evangelism
Version: 17 Branch → unspecified
I originally submitted this as a Moodle bug until I noticed it in more areas.
For reference: http://tracker.moodle.org/browse/MDL-35469
Dao/Gerv - what options do we have here besides evangelism? TinyMCE is used on a number of CMS sites, so I doubt that will actually be an effective way to mitigate the user impact.
tracking-firefox17: ? → +
tracking-firefox18: ? → +
Keywords: regressionwindow-wanted
(my guess is we don't have any other options since the UA is sent before we know whether TinyMCE is in use, but perhaps we should refresh the page with a different UA if we find an old TinyMCE version?)
What TinyMCE versions are affected?
(In reply to Dão Gottwald [:dao] from comment #5)
> What TinyMCE versions are affected?

That's a good question, but our QA is not in a position to do this testing.

Since bug 588909 is assigned to you Dao, please help find affected TinyMCE versions (to evaluate user impact), a forward fix (seems unlikely), or help us with a backout of bug 588909 for FF17.

If you do find the affected versions of TinyMCE, we can help by asking the blekko guys to run a "web grep" for us.

We'll likely back out bug 588909 from FF17 on 10/16 if we don't get clarity here - the user benefit of that bug is not great enough to fly blind here.
To start with, what lead to comment 4? http://demo.moodle.net/user/edit.php?id=5&course=1 doesn't seem to attempt using TinyMCE, which would mean that the detection happens on the server side, but AFAIK TinyMCE doesn't contain server-side components.
When I change the user agent string to contain Gecko/20100101, the TinyMCE scripts are actually loaded. They're using version 3.5.1.1, and it doesn't care about the date after "Gecko".
Summary: Rich Text Editor never loads → moodle serves plain textarea instead of rich text editor
Attached patch override the UA for moodle (obsolete) — — Splinter Review 
This lets us use the legacy UA when dealing with moodle.

Moodle is used at some universities at least in Germany, not all of which will be able to upgrade promptly even if moodle releases a fixed version in time even.
Comment on attachment 669123 [details] [diff] [review]
override the UA for moodle

No, sorry. This route is not acceptable. While "moodle" is perhaps a fairly unique string, I don't even want to start down this road of guessing what software people are using based on the URL. And I want UA overrides to be pref-controlled, not hard-coded.

Gerv
Attachment #669123 - Flags: review-
(In reply to Dão Gottwald [:dao] from comment #7)
> To start with, what lead to comment 4?
> http://demo.moodle.net/user/edit.php?id=5&course=1 doesn't seem to attempt
> using TinyMCE, which would mean that the detection happens on the server
> side, but AFAIK TinyMCE doesn't contain server-side components.

http://tracker.moodle.org/browse/MDL-35469
(In reply to Gervase Markham [:gerv] from comment #10)
> Comment on attachment 669123 [details] [diff] [review]
> override the UA for moodle
> 
> No, sorry. This route is not acceptable. While "moodle" is perhaps a fairly
> unique string, I don't even want to start down this road of guessing what
> software people are using based on the URL. And I want UA overrides to be
> pref-controlled, not hard-coded.
> 
> Gerv

It's a world full of compromises. Rather than pointing out the obvious (a guess is just a guess and can be wrong), can you propose something better?

As for prefs, we have a pref for disabling all overrides. That's sufficient for testing. I don't see us removing this override in a hot fix add-on, so I'm not sure what your concern is. Is it that we might want to *add* similar overrides without code changes?
I was pondering before making a suggestion as to what to do instead :-)

I don't see us removing any overrides in a hotfix addon, but the reason the system is flexible is that what is necessary is often unforeseen. I don't want to not have the ability to turn this off easily if it turns out to have unforeseen consequences, particularly as the patch is late-breaking for this release cycle.

Do we have any other instances where we change Firefox's behaviour based on the presence or absence of a string in the path? (I'll grant you our special behaviour for "ftp." in the case of the hostname.)

I think the first thing to do is get the bug properly reported at Moodle, get a fix developed, and then ask them about their expected deployment speed.

(If we do end up backing out the change, we should do so on desktop only. The Firefox for Android and B2G UAs should be left as they are.)

Gerv
Moodle is free -- you take it, you install it, you take care of it (or you don't). It doesn't have automatted updates, afaik. We can't count on manual updates, as universities are notoriously understaffed. One installation at my local university is at version 1.8.10 from 2009.

> Do we have any other instances where we change Firefox's behaviour based on
> the presence or absence of a string in the path? (I'll grant you our special
> behaviour for "ftp." in the case of the hostname.)

I don't know about paths, but Firefox on Android strips off the "m." prefix when the user wants to see the desktop version of a page, for instance. Anyway, I don't think sending the legacy UA string is a dramatic behavior change and I think it's preferable over a backout that would just mean sending the legacy string to the whole web.
I will say that looking for moodle in the URL will not catch all moodle instances.
The example I have on hand is online.unep.edu.au
(In reply to Hugh Nougher [:Hughman] from comment #16)
> I will say that looking for moodle in the URL will not catch all moodle
> instances.
> The example I have on hand is online.unep.edu.au

We can still evangelize or add individual overrides for specific instances (but only when we know about them). Do you have any idea how common this is? All instances I came across contained moodle in the URL, which could of course be a coincidence.
(In reply to Dão Gottwald [:dao] from comment #17)
> Do you have any idea how common this is?

I don's know. However using a quick Google search of two strings which are commonly part of Moodle theme I got over half of the results as not having Moodle in the URL.
See http://goo.gl/A9BBS
Ok, how about we override the UA string whenever we send a MoodleSession cookie?
That seems like a slightly less bad approach :-) But we should get in touch with the Moodle team to enquire as to what the best way is. Can people ever use this editor without being logged into Moodle, for example?

I've added a comment to their bug asking for someone to give us input.

Gerv
Thank you for taking our problem so seriously. Really we get what we deserve for relying on browser sniffing, but as Dao said above It's a world full of compromises.

It think looking for a cookie whose name starts MoodleSession is reliable. That is hard-coded
https://github.com/moodle/moodle/blob/master/lib/sessionlib.php#L162. Note that an arbitrary string chosen by the site administrator may be appended to that.
Sorry, I meant to add: I cannot think of a more reliable way to detect Moodle than this.
Depends on: 799502
Comment on attachment 669350 [details] [diff] [review]
override the UA for moodle (dependent on MoodleSession cookie)

moved to bug 799502
Attachment #669350 - Attachment is obsolete: true
So my understanding is that the issue has to do with TinyMCE, not specifically Moodle. Is there anything we can do to bandaid all affected TinyMCE editors? It's in use by a number of CMS flavors.
(In reply to Alex Keybl [:akeybl] from comment #25)
> So my understanding is that the issue has to do with TinyMCE, not
> specifically Moodle.

No, Moodle prevents TinyMCE from being loaded in the first place, see comment 7.
Hi, I'm the lead developer of Moodle.   Thanks all for working on this.

Thank you for considering the "mitigation" in Firefox based on the MoodleSession cookie to help our combined users.   We would also push our patch out to the Moodle community and educate people to upgrade ASAP but Firefox support (even for a while) would help a lot of schools without good technical support.

For Firefox code niceness, I think you could remove the patch from Firefox in a year or so.
(In reply to Hugh Nougher [:Hughman] from comment #0)
> These sites include Moodle and DotNetNuke.

Just so its known, when I said DotNetNuke at the start I had not actually checked around. The place I saw the problem was http://onyaktech.com/ forums. Since then I have looked at a few other DNN sites and the editor is working fine. So its likely only a small number of cases so I am not really worried.
(In reply to Martin Dougiamas from comment #27)
> We would also push our patch out to the Moodle community

Is the patch on track for being committed?
No longer blocks: 588909
Blocks: 799502
No longer depends on: 799502
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
> Is the patch on track for being committed?

Just to comment this patch was committed today.
Product: Tech Evangelism → Tech Evangelism Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: