Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Override the UA string for moodle to get the rich text editor rather than plain textareas

REOPENED
Unassigned

Status

()

Firefox
General
REOPENED
5 years ago
5 years ago

People

(Reporter: dao, Unassigned)

Tracking

Trunk
Firefox 19
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17+ verified, firefox18+ fixed)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 669535 [details] [diff] [review]
patch

+++ This bug was initially created as a clone of Bug #797703 +++

see bug 797703 for details
Attachment #669535 - Flags: review?(gerv)
Attachment #669535 - Flags: review?(felipc)
Attachment #669535 - Flags: review?(bzbarsky)
(Reporter)

Updated

5 years ago
Component: English US → General
Product: Tech Evangelism → Firefox
Version: unspecified → Trunk

Comment 1

5 years ago
Comment on attachment 669535 [details] [diff] [review]
patch

r=me

I hate the web.  :(
Attachment #669535 - Flags: review?(bzbarsky) → review+
Comment on attachment 669535 [details] [diff] [review]
patch

r=gerv, echoing bz's opinion. If it were possible to check for cookies beginning with "MoodleSession" rather than containing it, that would be even better. I believe JS now has a startsWith?

Can you quickly outline what you did to test this?

Gerv
Attachment #669535 - Flags: review?(gerv) → review+
(Reporter)

Comment 3

5 years ago
(In reply to Gervase Markham [:gerv] from comment #2)
> Comment on attachment 669535 [details] [diff] [review]
> patch
> 
> r=gerv, echoing bz's opinion. If it were possible to check for cookies
> beginning with "MoodleSession" rather than containing it, that would be even
> better. I believe JS now has a startsWith?

The Cookie header contains multiple cookies here, so it's more complicated than that.

> Can you quickly outline what you did to test this?

I loaded http://demo.moodle.net/user/edit.php?id=5&course=1 and logged in as admin.
OK, fair enough on the cookie name.

Gerv
Comment on attachment 669535 [details] [diff] [review]
patch

Review of attachment 669535 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.  I'd prefer having "UserAgentOverrides.init();" + the added block in a separate function to keep OnProfileStartup clean, but it's up to you.
Better yet I think would be to have this specific knowledge about Moodle (and other future cases?) in UserAgentOverrides.jsm instead of nsBrowserGlue, but I assume you did it this way to keep this Desktop-only.
Attachment #669535 - Flags: review?(felipc) → review+
(Reporter)

Comment 6

5 years ago
Created attachment 669933 [details] [diff] [review]
patch v2

(In reply to :Felipe Gomes from comment #5)
> Looks good to me.  I'd prefer having "UserAgentOverrides.init();" + the
> added block in a separate function to keep OnProfileStartup clean, but it's
> up to you.

done

> Better yet I think would be to have this specific knowledge about Moodle
> (and other future cases?) in UserAgentOverrides.jsm instead of
> nsBrowserGlue, but I assume you did it this way to keep this Desktop-only.

exactly
Attachment #669535 - Attachment is obsolete: true
(Reporter)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bead66674f12
moving tracking to this bug, removing it from 797703
status-firefox17: --- → affected
status-firefox18: --- → affected
tracking-firefox17: --- → +
tracking-firefox18: --- → +

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/bead66674f12
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
(Reporter)

Comment 10

5 years ago
Comment on attachment 669933 [details] [diff] [review]
patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 588909
User impact if declined: plain textarea instead of rich text editor on moodle (worse user experience)
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): reasonably low risk, and the User Agent override for moodle can be disabled via a pref
String or UUID changes made by this patch: none
Attachment #669933 - Flags: approval-mozilla-beta?
Attachment #669933 - Flags: approval-mozilla-aurora?
Is this feature really need?
The previous similar patch supporting aol.com is even so, I don't think these specified patch is good way.
These approach is domain specific. If this approach is recognized,  other sites in the world have the right to get a same support.

If we need to land this support necessarily, we should take more versatile method (e.g. site specfic option).
(Reporter)

Comment 12

5 years ago
(In reply to OHZEKI Tetsuharu [:saneyuki_s] from comment #11)
> Is this feature really need?

Yes, see the discussion in bug 797703. We expect that many Moodle instances won't be updated soon and we don't want a worse experience on Moodle for Firefox users.
(In reply to Dão Gottwald [:dao] from comment #12)
> Yes, see the discussion in bug 797703. We expect that many Moodle instances
> won't be updated soon and we don't want a worse experience on Moodle for
> Firefox users.

OK. I understand the how and why.

However, I don't like this approach...... Even if it's a temporary treatment, this approach is special treatment for a specified site. It may be not browser's business as a Web-platform.

Comment 14

5 years ago
Moodle is a web application that lots of universities (and other people) install on their own servers. So, adding a work-around for Moodle is like adding a work-around for Drupal or Wordpress. It is not like adding a work-around for Twitter or Facebook.

Comment 15

5 years ago
No one _likes_ this approach.

Do feel free to suggest a better one if you have one....
Tim, Boris, Thank you for your accounts.

At comment 13, I did not have a mind to go against this approach strongly.
I apologize that my comment dragged up the discussion.
Comment on attachment 669933 [details] [diff] [review]
patch v2

Thanks guys - I know this isn't the perfect solution, but it will definitely make our users have a much better experience.
Attachment #669933 - Flags: approval-mozilla-beta?
Attachment #669933 - Flags: approval-mozilla-beta+
Attachment #669933 - Flags: approval-mozilla-aurora?
Attachment #669933 - Flags: approval-mozilla-aurora+
(Reporter)

Comment 18

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/b8c17cd78859
https://hg.mozilla.org/releases/mozilla-beta/rev/bc0e8935942a
status-firefox17: affected → fixed
status-firefox18: affected → fixed
Keywords: verifyme
(Reporter)

Updated

5 years ago
(Reporter)

Updated

5 years ago
Blocks: 588909
No longer blocks: 797703
Depends on: 797703

Comment 19

5 years ago
A big thanks from Moodle for implementing this workaround. We've integrated a fix for the underlying problem in Moodle (http://tracker.moodle.org/browse/MDL-35469), but it'll take sites some time to update with this fix.
Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Firefox/17.0

Setting to verified in F17 beta 5. Checked on Windows 7, Mac OS 10.7, Ubuntu 12.04. 
Rich text editor displayed as expected and functional on http://demo.moodle.net/user/edit.php?id=5&course=1

The site's layout is broken in Firefox however. Logged bug 810828.
status-firefox17: fixed → verified

Comment 21

5 years ago
This change breaks another feature, linking Moodle with Mahara using MNet: http://docs.moodle.org/23/en/MNet 

MNet allows users of a Moodle site to roam to a Mahara site. The atuhentication process checks if the User Agent string received by both sites are equal to allow access.
(In reply to albert.gasset from comment #21)
> This change breaks another feature, linking Moodle with Mahara using MNet:
> http://docs.moodle.org/23/en/MNet 
> 
> MNet allows users of a Moodle site to roam to a Mahara site. The
> atuhentication process checks if the User Agent string received by both
> sites are equal to allow access.

That sounds like tech evangelism to Mahara then - as it is a website and not an application, please file a separate bug for Tech Evangelism.

Comment 23

5 years ago
Mahara is an application, it's an open source portfolio system that is commonly used with Moodle. This is causing a serious problem for its users: https://bugs.launchpad.net/mahara/+bug/1082416?comments=all

I understand this workaround was implement to avoid trouble for some users, but for other users this is even worse. Why not fix the problem at the right place (Moodle), to avoid the risk of more side effects?

(In reply to Lukas Blakk [:lsblakk] from comment #22)
> (In reply to albert.gasset from comment #21)
> > This change breaks another feature, linking Moodle with Mahara using MNet:
> > http://docs.moodle.org/23/en/MNet 
> > 
> > MNet allows users of a Moodle site to roam to a Mahara site. The
> > atuhentication process checks if the User Agent string received by both
> > sites are equal to allow access.
> 
> That sounds like tech evangelism to Mahara then - as it is a website and not
> an application, please file a separate bug for Tech Evangelism.

Comment 24

5 years ago
Hi Albert,

In Moodle, we have now fixed this ( http://tracker.moodle.org/browse/MDL-35469 ), however i'm afraid its not likely at all that all Moodle sites will be upgraded with this fix.
(In reply to albert.gasset from comment #21)
> MNet allows users of a Moodle site to roam to a Mahara site. The
> atuhentication process checks if the User Agent string received by both
> sites are equal to allow access.

The obvious question is: what on earth for? Given that you have a proper SSO system, this provides precisely zero added security.

If it's the considered opinion of the Moodle leadership that we should remove the workaround, we will. You have more info than us to make the decision on which problem is worse.

Gerv

Comment 26

5 years ago
Can we simply send the same cooked-up UA to Mahara?  Can we detect when we're dealing with it?
Blocks: 815446
I filed bug 815446 for the MNet SSO fallout.
Partly backed out by bug 815743; rest needs cleanup -> reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 29

5 years ago
(In reply to Ed Morley [UTC+0; email:edmorley@moco] from comment #28)
> Partly backed out by bug 815743; rest needs cleanup -> reopening.

Could you be more specific? What's the rest and what kind of cleanup is needed?
(Reporter)

Updated

5 years ago
Keywords: verifyme
(Reporter)

Updated

5 years ago
Assignee: dao → nobody
You need to log in before you can comment on or make changes to this bug.