[MDL-34068] chat daemon not sending response headers to browser. Breaks Firefox - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 1.9.18, 2.2, 2.2.5, 2.3.2, 2.4
Fix Version/s: 2.2.6, 2.3.3
Component/s: Chat
Labels:
- patch
- triaged

Testing Instructions:
Hide

This issue must be tested with ~~MDL-27398~~ integrated and with different browsers (the issue was reported with FireFox)

In Admin tree -> Plugins -> Activity modules -> Chat, select "Chat server daemon" as chat method

Go to a course and create a chat activity instance

Go to the chat instance and follow "Click here to enter the chat now"

You SHOULD be able to see a popup divided in 3 parts, you SHOULD NOT see as plain text (with HTML tags) in the upper-left side, you SHOULD see the user picture
Show
This issue must be tested with MDL-27398 integrated and with different browsers (the issue was reported with FireFox) In Admin tree -> Plugins -> Activity modules -> Chat, select "Chat server daemon" as chat method Go to a course and create a chat activity instance Go to the chat instance and follow "Click here to enter the chat now" You SHOULD be able to see a popup divided in 3 parts, you SHOULD NOT see as plain text (with HTML tags) in the upper-left side, you SHOULD see the user picture
Affected Branches:
MOODLE_19_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
Fixed Branches:
MOODLE_22_STABLE, MOODLE_23_STABLE
Pull from Repository:
git://github.com/dmonllao/moodle.git
Pull Master Branch:
~~MDL-34068~~_master
Pull Master Diff URL:
https://github.com/dmonllao/moodle/compare/master...MDL-34068_master

Rank:
42373

Description

When you run the chat daemon, which uses a frameset, the main chat area, (where messages appear, i.e., win=chat) does not send out a response header. Proper response headers are sent for the sidekick frames (message, beep, and user). The user sees the underlying html exposed in Firefox. Firefox treats the output in quirks mode and as text/plain not text/html. IE Chrome appear to be more forgiving.

I tracked this down using Fiddler, Firefox' Live Headers add-on and by looking at the Firefox cache (about:cache).

As a partial fix, I added an HTTP response block beginning on line 532 of chatd.php. However, this may cause some of the sidebars to send the header response twice.

chatd.php

$this->dismiss_half($sessionid, false);

        // new block begins here

        $header  = "HTTP/1.1 200 OK\n";
        $header .= "Date: ".date('r')."\n";
        $header .= "Server: Moodle\n";
        $header .= "Content-Type: text/html; charset=utf-8\n";
        $header .= "Last-Modified: ".gmdate("D, d M Y H:i:s")." GMT\n";
        $header .= "Cache-Control: no-cache, must-revalidate\n";
        $header .= "Expires: Wed, 4 Oct 1978 09:32:45 GMT\n";
        $header .= "\n";
        $this->write_data($this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL], $header);

        // new block ends here

        $this->write_data($this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL], $CHAT_HTMLHEAD_JS);

Issue Links

is blocked by

MDL-27398 Chat daemon doesn't run properly

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Matteo Scaramuccia added a comment - 28/Jun/12 2:12 AM

FYI: this has been discussed in http://moodle.org/mod/forum/discuss.php?d=205471, before filing the issue.

Show

Matteo Scaramuccia added a comment - 28/Jun/12 2:12 AM FYI: this has been discussed in http://moodle.org/mod/forum/discuss.php?d=205471 , before filing the issue.

Hide

Permalink

Michael de Raadt added a comment - 28/Jun/12 4:16 PM

It looks like this is still probably a problem with the current version.

Show

Michael de Raadt added a comment - 28/Jun/12 4:16 PM It looks like this is still probably a problem with the current version.

Hide

Permalink

David Monllaó added a comment - 06/Sep/12 9:55 AM

Thanks Paul for reporting the issue and providing a solution.

I've added text/html headers to the new connection output, but I can't see any duplicate response header in any of the other frames, each of them has it's own header and this new addition does not conflict. I avoid investing more efforts in this issue (refactoring code I mean) because, if it has to be done, should be part of a general cleanup of the chat daemon code.

Show

David Monllaó added a comment - 06/Sep/12 9:55 AM Thanks Paul for reporting the issue and providing a solution. I've added text/html headers to the new connection output, but I can't see any duplicate response header in any of the other frames, each of them has it's own header and this new addition does not conflict. I avoid investing more efforts in this issue (refactoring code I mean) because, if it has to be done, should be part of a general cleanup of the chat daemon code.

Hide

Permalink

Rossiani Wijaya added a comment - 06/Sep/12 3:09 PM

This looks good.

+1 for integration.

Show

Rossiani Wijaya added a comment - 06/Sep/12 3:09 PM This looks good. +1 for integration.

Hide

Permalink

David Monllaó added a comment - 06/Sep/12 3:20 PM

Thanks Rossie

Show

David Monllaó added a comment - 06/Sep/12 3:20 PM Thanks Rossie

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 07/Sep/12 9:34 AM

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Show

Eloy Lafuente (stronk7) added a comment - 07/Sep/12 9:34 AM The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao

Hide

Permalink

David Monllaó added a comment - 07/Sep/12 12:15 PM

Rebased

Show

David Monllaó added a comment - 07/Sep/12 12:15 PM Rebased

Hide

Permalink

Aparup Banerjee added a comment - 10/Sep/12 1:44 PM

Thanks, thats been integrated.

Show

Aparup Banerjee added a comment - 10/Sep/12 1:44 PM Thanks, thats been integrated.

Hide

Permalink

Ankit Agarwal added a comment - 12/Sep/12 3:39 PM

Tested this with random browsers and themes. All working good.
Thanks

Show

Ankit Agarwal added a comment - 12/Sep/12 3:39 PM Tested this with random browsers and themes. All working good. Thanks

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 15/Sep/12 2:38 AM

Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio

This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

Thanks!

Show

Eloy Lafuente (stronk7) added a comment - 15/Sep/12 2:38 AM Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!

People

Assignee:

David Monllaó

Reporter:

Paul Kissman

Peer reviewer:

Rossiani Wijaya

Integrator:

Aparup Banerjee

Tester:

Ankit Agarwal

Participants:

Ankit Agarwal, Aparup Banerjee, David Monllaó, Eloy Lafuente (stronk7), Matteo Scaramuccia, Michael de Raadt, Paul Kissman, Rossiani Wijaya

Votes:

0 Vote for this issue

Watchers:

2 Start watching this issue

Dates

Created:

28/Jun/12 1:26 AM

Updated:

30/Sep/13 1:07 PM

Resolved:

15/Sep/12 2:38 AM

Agile

View on Board