Summary | Forward / Reply / New message results in popup "Loading..." |
Queue | IMP |
Queue Version | 6.2.12 |
Type | Bug |
State | Resolved |
Priority | 1. Low |
Owners | mrubinsk (at) horde (dot) org |
Requester | horde-support (at) inetspec (dot) com |
Created | 02/24/2016 (3422 days ago) |
Due | |
Updated | 10/20/2017 (2818 days ago) |
Assigned | 02/26/2016 (3420 days ago) |
Resolved | 03/18/2016 (3399 days ago) |
Github Issue Link | |
Github Pull Request | |
Milestone | |
Patch | No |
commit 4581143a7c784102f4976645715ea2bab9e21024
Author: Michael J Rubinsky <mrubinsk@horde.org>
Date: Fri, 18 Mar 2016 07:14:21 -0400
Bug: 14267Cleaner way of fixing opening compose window on IE whenuploads are disabled.
M js/compose-dimp.js
https://github.com/horde/imp/commit/4581143a7c784102f4976645715ea2bab9e21024
commit 3f2a8628a7ea3f541f7da60a830702c6dd4cbb95
Author: Michael J Rubinsky <mrubinsk@horde.org>
Date: Fri, 18 Mar 2016 07:12:51 -0400
Revert "
Bug: 14267Fix opening compose window on IE when uploads aredisabled."
This reverts commit 6d45affb6b861786924630eefc2c1b758d6d07bc.
M js/compose-dimp.js
https://github.com/horde/imp/commit/3f2a8628a7ea3f541f7da60a830702c6dd4cbb95
commit 8faee7d4b14191bc75aecab19c65ffbe9f216675
Author: Michael J Rubinsky <mrubinsk@horde.org>
Date: Fri, 18 Mar 2016 00:22:28 -0400
Bug: 14267Fix opening compose window on IE when uploads are disabled.M js/compose-dimp.js
https://github.com/horde/imp/commit/8faee7d4b14191bc75aecab19c65ffbe9f216675
you think that mod will be available in the upstream through PEAR?
charm. Excellent! and Thank You!
I want to leave the manual change in our server for now. When do you
think that mod will be available in the upstream through PEAR?
commit 7361c03d08e076ecc226711f50ceda2627584c56
Author: Michael J Rubinsky <mrubinsk@horde.org>
Date: Fri Mar 18 07:14:21 2016 -0400
Bug: 14267Cleaner way of fixing opening compose window on IEwhen uploads are disabled.
imp/js/compose-dimp.js | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
http://github.com/horde/horde/commit/7361c03d08e076ecc226711f50ceda2627584c56
commit 3f9bb5f74e805a8b6520ba21258a5b33ff5969b1
Author: Michael J Rubinsky <mrubinsk@horde.org>
Date: Fri Mar 18 07:12:51 2016 -0400
Revert "
Bug: 14267Fix opening compose window on IE when uploadsare disabled."
This reverts commit 6d45affb6b861786924630eefc2c1b758d6d07bc.
imp/js/compose-dimp.js | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
http://github.com/horde/horde/commit/3f9bb5f74e805a8b6520ba21258a5b33ff5969b1
Assigned to Michael Rubinsky
commit 6d45affb6b861786924630eefc2c1b758d6d07bc
Author: Michael J Rubinsky <mrubinsk@horde.org>
Date: Fri Mar 18 00:21:12 2016 -0400
Bug: 14267Fix opening compose window on IE when uploads are disabled.imp/js/compose-dimp.js | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
http://github.com/horde/horde/commit/6d45affb6b861786924630eefc2c1b758d6d07bc
New Attachment: Horde-Server-compose-PLAINTEXT.html
text compose window.
After collecting the source DOM from the demo site and this server for
the compose window, and normalizing the domains, usernames, session
variables, etc. for comparison;
There is one primary difference that sticks out: the sections that
enable attachments are empty in the one from this server.
The following is missing in line 26 of the DOM:
<input id="save_attachments_select" name="save_attachments_select"
type="hidden" value="0" /> <input id="MAX_FILE_SIZE"
name="MAX_FILE_SIZE" type="hidden" value="2097152" />
and, at line 100 of the DOM, what should look like:
<td class="label">
<span class="iconImg attachmentImg"></span>:
</td>
<td>
<div id="atcdrop" style="display:none">
Drop file(s) here to attach. </div>
<div id="atcdiv">
<span id="upload_limit" style="display:none">The attachment
limit has been reached.</span>
<span id="upload_wait" style="display:none"></span>
<span>
<label id="compose_upload_add" for="upload">Add Attachment</label>
<input id="upload" multiple name="file_upload[]" type="file"
/> </span>
actually looks like:
<td></td>
<td>
<div>
Would this explain the results in the OP since the compose form does
not have the same structure?
The only other differences seem to be related to language selection
and the spell checker. But those are preferences, right?
Will wait for text/html and Pear library list for comparison...
mean they are the same issue. Some of those bug reports are ancient,
some involve only Macs (no clue how that related to IE detection), and
none of them are related to the same code your bug report suggested
was the issue.
attached in that line actually works correctly and catches the events
it is intended to or are you just stating that after the page loaded
there were no errors?
errors.
that everything was working. If the event handlers that are being
registered do not work, then things are NOT "just fine".
send a message by allowing the code to believe that it was serving to a
browser other than IE.
Honestly, I am not even sure what events that I should be checking for
other than the ability to avoid a fatal error. Care to clue me in what
you are looking for here? Hovers, email lookups, drag and drop, what?
question since you had such a thorough explanation. It's the 'change'
handler of the 'upload' and 'identity' node...but this is rather moot
anyway, since we know that those are not registered correctly on
certain versions of IE and thus we cannot remove that code as you
suggest.
server to find this, as I also tested using this. demo.horde.org
you are using so that I can also compare against our installation?
current Git master code, and the Horde demo server.
side we need more information. As you state, many people use IE and
so far this is the one and only report of this.
browsers have evolved to treat various DOMs differently.
supported in the same major version.
Line 2092 InvalidCharacter
It sounds to me like everything else afterwards is a side effect of this.
has been in place for years.
other than this one, newest to oldest:
https://bugs.horde.org/ticket/13313
https://bugs.horde.org/ticket/13079
https://bugs.horde.org/ticket/12916
https://bugs.horde.org/ticket/10187
https://bugs.horde.org/ticket/8740
attached in that line actually works correctly and catches the
events it is inteded to or are you just stating that after the page
loaded there were no errors?
errors. I only stated that the compose window did load and was able to
send a message by allowing the code to believe that it was serving to a
browser other than IE.
Honestly, I am not even sure what events that I should be checking for
other than the ability to avoid a fatal error. Care to clue me in what
you are looking for here? Hovers, email lookups, drag and drop, what?
determined what needs fixing and the best way to fix it so as to not
introduce a regression for any other browsers.
should look elsewhere. I would suggest looking at the DOM structure as
well as the library versions from PEAR.
Can you provide a text/html of a working New Message window popup that
I can check against the window popup content on my side?
Can you provide a PEAR list of libraries w/ versions on the server that
you are using so that I can also compare against our installation?
As you state, many people use IE and so far this is the one and only
report of this.
the same results.
other different platforms since the original post and am not convinced
the problem is limited to IE at all.
It seems to me that with the number of un-resolved reports of similar
problems and the consistency of the errors in specific installations
that, the problem may actually be related to:
1. an un-documented dependency in a PEAR library, or
2. an un-tested dependency for a later version of a PEAR library
It may even be something that has developed over a period of time as
browsers have evolved to treat various DOMs differently.
I have looked at the installation logs from the server that we are
using on this installation. I do not see any 'required' libraries
missing and there are no complaints about versioning that I can see.
With specific regard to IE11, it looks much more like Mozilla than it
does the old IE. Similarly with Edge, it looks much more like
WebKit than it does anything else. Therefore, in those two cases,
the calls that are more appropriate for more modern browsers would
make more sense than the calls for say, IE7 or IE8.
Furthermore, it may actually be the DOM structure that is confusing
the browser on the client side, and there are many factors that
adjust and change that structure for the dynamic views, especially
when the dependencies on various PEAR libraries are taken into
account.
If we can identify where the base problem is introduced and find an
applicable solution, we will probably solve all of the outstanding
tickets related to this compose window popup problem.
So the only thing we have proven and tested is that two (or more)
slight variants of IE11 interpret line 1411 differently? I don't
see how that proves that the code is logically sound.
has been in place for years.
line 1413 works just fine in IE 11?
attached in that line actually works correctly and catches the events
it is inteded to or are you just stating that after the page loaded
there were no errors? This is not the same thing.The reason for that
code is that events do not bubble up properly in IE. Whether they do
this properly in some newer version of IE is irrelevant (and to be
clear, I have not verified this one way or the other yet). Just
because a newer version of IE may work correctly doesn't mean we
should remove the check for older versions within the same major
release cycle. We are not going to remove support for a browser that
worked properly within the same major release.
determined what needs fixing and the best way to fix it so as to not
introduce a regression for any other browsers.
sense to support only the version that one developer has tested with.
feedback to your reports in an effort to help you further drill down
on where the actual issue is. Since we cannot reproduce this on our
side we need more information. As you state, many people use IE and so
far this is the one and only report of this.
Additionally, I have also tested this on IE 11.0.9600.17351 - with the
same results.
So the only thing we have proven and tested is that two (or more)
slight variants of IE11 interpret line 1411 differently? I don't see
how that proves that the code is logically sound.
Why is the IMP code even checking for that difference for IE11 if line
1413 works just fine in IE 11? If it is looking just because IE6, 7,
8, etc. had a problem, then that is no longer a logical thing to look
for. All of those browsers are obsolete.
I have given plenty of excellent feedback on this issue and performed
the same test on multiple machines with the same results.
If noone chooses to fix the bug, so be it, I will edit code on the
server to work for the time being. Too many people use IE11 to ignore
the problem, and it doesn't make sense to support only the version
that one developer has tested with.
best of luck
then at line 1409, your browser is not being detected as IE by the
prototype.js code.
returns true right before the conditional you are referring to. I.e.,
my browser is definitely being detected as IE.
your browser is detected as "Mozilla", then your browser skips
around the code that crashes other IE browsers (but perhaps not
all). As it is, in our local case, the browsers are definitely
executing line 1411 and not line 1413 despite the newer user agent
string.
$('upload') are not resolving to the window object, but rather to the
correct DOM element.
both IE11 (11.103.10586.0) and Edge on Windows 10.
Are you using JS caching? If so, try it with caching turned off and
then with it on, but after clearing the cache files.
IE is a slightly different version, but we do not really have control
over the version that we use as it is based on administrative updates.
The version on this machine is currently 11.0.10240.16683.
I cleared the cache from IE and tried again without any change in results.
I then logged out, cleared the IE cache, restarted the browser, and
logged into "basic" mode. Still gives a number of different errors in
the console, but no fatal ones. I was able to send a test email in
Basic mode.
I then repeated those steps for Dynamic mode. After verifying that I
had no cached .js files and restarting the browser, I logged in the
same user as Dynamic mode. I received the same results as the
original ticket.
At any rate, I see no evidence of cached .js files on the client side.
Is "JS caching" something that is configured by default in Horde or
IMP? If so, how is that default changed? Again, at any rate, I have
not been able to find any evidence of cached files in the Horde
installation or the HTTP server, so this is probably a 'moot' point
More directly, looking at the call stack and the trace, the difference
in handling between IE and other browsers occurs at lines 1408-1414 in
the ./imp/js/compose-dimp.js file. It looks like that call is
directly related to the resulting error. It is interesting that it
mentions "bubble" in the comment.
A window object is being passed into prototype.js but the window does
not have an 'observe' method. This would normally be a 'document'
object which does have an 'observe' method.
I temporarily edited the ./imp/js/compose-dimp.js file to allow IE to
be treated the same an any other browser, then cleared the browser
cache, and logged in again using Dynamic mode. I was then able to
begin a new message and successfully send.
1408 /* Attach event handlers. */
1409 if (Prototype.Browser.IE) {
1410 // IE doesn't bubble change events.
1411 // $('identity', 'upload').invoke('observe', 'change',
this.changeHandler.bindAsEventListener(this));
added-> document.observe('change',
this.changeHandler.bindAsEventListener(this));
1412 } else {
1413 document.observe('change',
this.changeHandler.bindAsEventListener(this));
1414 }
Apparently, on line 1411 the expression "('identity', 'upload')"
resolves to the window, and not the document (at least in IE11).
I cannot find any specific revision information inside the
compose-dimp.js file, however, it's timestamp matches all of the other
.js files for IMP from the date of installation which was Feb 5. (at
least it did until I edited the file to perform the test)
The prototype.js file shows: Prototype JavaScript framework, version
1.7.2, and a file date of Feb 23 (which was the last time that I did a
pear update). However, this problem existed previously, which is why
I did the pear update. There is a newer (1.7.3) prototype.js
available from the upstream. I would be happy to test with that.
However, it looks like the root of the problem is the 'if' decision
made in the compose-dimp.js file at line 1409.
Likely causes may be:
- differing user-agent strings / features reported from various
versions of IE
- differing DOM structures in various versions of IE
- JScript interpretation of Prototype JS at various versions
The browser on this machine is currently reporting:
"Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; rv:11.0) like Gecko"
Also, I would suggest that if you cannot reproduce this problem, then
at line 1409, your browser is not being detected as IE by the
prototype.js code. That would explain the difference in results. If
for any reason your browser is detected as "Mozilla", then your
browser skips around the code that crashes other IE browsers (but
perhaps not all). As it is, in our local case, the browsers are
definitely executing line 1411 and not line 1413 despite the newer
user agent string.
For what it's worth, I think this IE detection issue is still open on
GitHub at the time of this writing...
Ref: https://github.com/sstephenson/prototype/issues/120
So, what does :: if (Prototype.Browser.IE) :: really mean? I am not
conviced that it means anything related to whether a browser supports
"bubble" or "capture" for events, at least not in today's world.
Should that handling call really be based on that specific boolean
flag in Prototype JS?
I believe that this warrants a closer look into how the calls are made
from Horde / IMP code into supporting libraries, especially when they
are done differently based on a decision about which User Agent is in
use.
Is there a better way to address the 'document.observe' method with a
more cross browser compatible statement, or is it just a browser
detection problem?
It may be necessary to further detect versions and capabilities when
IE is the User Agent in order to avoid filing IE11 into the "do it
differently" column.
Again, very happy to help with more information, and if I was better
with Prototype JS / Javascript I would even try hacking at a permanent
solution. If you are using IE11, then you could try forcing it to run
line 1411 and see what happens.
Or, is that code even the same in your currently used version of
compose-dimp.js?
Priority ⇒ 1. Low
State ⇒ Feedback
IE11 (11.103.10586.0) and Edge on Windows 10.
Are you using JS caching? If so, try it with caching turned off and
then with it on, but after clearing the cache files.
Priority ⇒ 3. High
State ⇒ Unconfirmed
New Attachment: 969-UnableToGetProperty-FATAL.PNG
Patch ⇒ No
Milestone ⇒
Queue ⇒ IMP
Summary ⇒ Forward / Reply / New message results in popup "Loading..."
Type ⇒ Bug
Edge) results in the popup window hanging with the "Loading..."
message indefinitely.
Server Environment:
CentOS 6 - Apache - MySQL - PHP 5.3.3
Horde Groupware Webmail Edition 5.2.12 (installed via PEAR)
Mail (imp) 6.2.12
Browser: IE11 (also Edge), have not tested on IE8 - IE10
Note: Does NOT occur on Firefox or Chrome and is not limited to any
single computer installation. We have tested on Windows 8.1 Pro,
Windows 7, and Windows 10 machines.
Using the Debugger in IE shows multiple errors occuring through the
prototype.js file. The last one being fatal and stopping the script.
I will list the errors in the order that they occur:
Line 2092 InvalidCharacter
Line 5032 SyntaxError
Line 5045 SyntaxError
Line 969 Unable to get property 'observe' of undefined or null reference
The Call Stack at the point of the final error looks like this:
Unable to get property 'observe' of undefined or null reference
[Main Thread]
Anonymous function [Line: 969, Col: 7], prototype.js
Anonymous function [Line: 1270, Col: 9], prototype.js
invoke [Line: 968, Col: 5], prototype.js
DimpCompose.onDomLoad [Line: 1411, Col: 13], compose-dimp.js
Anonymous function [Line: 7206, Col: 7], prototype.js
fireEvent_DOM [Line: 7066, Col: 5], prototype.js
fire [Line: 7055, Col: 5], prototype.js
_methodized [Line: 456, Col: 7], prototype.js
fireContentLoadedEvent [Line: 7241, Col: 5], prototype.js
[Async Call]
Anonymous function [Line: 7269, Col: 5], prototype.js
Global code [Line: 7231, Col: 2], prototype.js
We understand that Firefox / FireBug are preferred for this type of
troubleshooting. However, these errors simply do not surface when
using either Firefox or Chrome.
We have attached a screen snippet and will be happy to provide any
further information we can in order to track down this problem.
Thanks,
Jon