How to reproduce?
Using IE6 open any page that contains a textarea with the resizable attribute and click the textarea itself (not the grippie). As soon as you release the mouse button, you get a "not implemented" javascript error in line 102.

Why doesn it happen?
Because the onmouseup event handler is installed for the textarea itself when it shouldn't. Note it is already installed by the onmousedown event handler of the grippie.

How to fix?
Open misc/textarea.js and remove the following line:

this.element.onmouseup = function (e) { ta.endDrag(e); };

CommentFileSizeAuthor
#4 textarea.js.patch43911a494 bytesmarkus_petrux
#3 textarea.js3.23 KBCrell
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Priority: Critical » Normal

Can you roll a patch that does as you suggest? Thanks. (Same for the other JS issues you just reported.)

http://drupal.org/diffandpatch

markus_petrux’s picture

I know this would be kind of offtopic here, but...

could you please explain me why do you need a patch for such a simple fix?

It is not because I want to do it "my way", it is just I don't understand why (in certain cases where a fix is so simple) devs ask for patches when that requires tools that the reporter could not have at hand. Even if I report a patch, I guess the maintainer should still test the fix, 'cause I might be wrong. ...or is it because devs use tools that require patches to apply even simple fixes? Sorry, I have never seen this methodology before. Thanks

Crell’s picture

Status: Active » Needs review
FileSize
3.23 KB

As I see it, a couple of reasons (although I am not the Project Leader nor even one of the big people, so please don't take anything I say as a "party line", and Dries et al may have different reasons).

1) We try to keep everything done via patches rather than directly editing the code for easier tracking of who did what and what side effects there were. It makes everything easy to back out, as well.

2) People who aren't really developers but can run one or two simple commands can still test patches to see if they work properly, but only if the fix is provided in patch form rather than "edit this file" instructions.

3) Patches are more definitive. A patch file specifies "line 473" precisely and will automatically keep track of that line moving if something changes elsewhere in the file. "edit this file" instructions do not.

4) Dries (the Project Leader) tends not to look at a patch until someone else has already reviewed it and set it "Ready to Commit". Given the number of patches and bug reports that come in, he'd go crazy if he tried to read everything. Using patches makes it easier for the other contributors to review patches before they get to him, so that by the time it does get to him it's rock solid.

True, not everyone has the tools. They are easy and free to get, though, regardless of your OS. The link I provided and the pages after it show how. If you're going to be reporting how to fix more than one or two bugs, then it's probably a good idea to set that up so that you can help fix bugs you know how to fix. It makes the bugs get fixed faster, frees up other people to fix other things, and gets you involved in a super-cool open source project that could always use more smart people fixing things. :-)

That said, I've attached a patch that does as markus suggests. I've not tested it at all, since I'm on Linux, but someone please do so.

Also, I've not been paying much attention to Drupal's JS functionality, but it looks like all the events are being assigned with element.onsomething = function foo() {}; Isn't that the sloppy way? I thought the proper way to assign event handlers was with element.addEventListener(), (with a proper wrapper to account for IE's stupid attachEvent() function as well). Steven, any reason you did it the less other-code-friendly way?

markus_petrux’s picture

FileSize
494 bytes

Thanks, Crell, for the explanation. Much appreciated :-) (it is point 4 the one that makes sense to me).

Though, you have uploaded the whole file, I'm attaching the patch for this single fix.

As per the comment about attaching event handlers... AFAIK, the "traditional" method (used by drupal now, here and the onload wrapper) works on all browsers. Working with standards in mind is good and it should be the way to go, but when it comes to javascript it is not always possible (sadly). No one seems to be able to stop browser makers to create their own extensions/methods. There's more information here:

http://www.quirksmode.org/js/events_tradmod.html

Cheers

Crell’s picture

Oh expletive deleted! Figures I'd do something dumb like that after that long lecture. :-)

As for the Javascript style, the problem with using .onevent is that you can register one and only one handler that way. Given that Drupal is based around dozens of modules interacting in a friendly way, that is decidedly unfriendly. addEventListener() is multi-handler friendly. There are, also, common best practices for handling IE and Safari (which break the standard in two different ways) that we should be using.

http://www.sitepoint.com/books/dhtml1/ - This book covers many of them (it's where I learned about them), and the sample chapters include the basic techniques and functions so you don't have to spend any money to see what I'm talking about. :-)

markus_petrux’s picture

The current approach implemented in drupal is unobstrusive and works on all browsers (at least those that pass the isJsEnabled() check). Note the onmousedown event handler used here is attached to a dynamically created element. If any other JS snippet loaded at a later time, needs to do something with it, it is its own responsability to degrade nicely and/or don't break its functionality. As an exemple, the way the onmousemove and onmouseup event handlers are attached to the document object here. The old handler is saved before and then restored when the drag operation ends.

Using the DOM method is not possible because it would break browsers that do not support it yet, therefore it would need a wrapper to manage events. This is probably a good thing to do, but it would need time and test on all possible platforms.

Steven’s picture

Status: Needs review » Fixed

That line was left-over from me trying to fix an Opera issue. It seems Opera does not bubble through onmouseup events from textareas. Committed to HEAD.

Crell: There are several issues with event handlers, like e.g. how to deal with multiple return values. We have simple APIs (addLoadEvent, addSubmitEvent) for such messy situations. However, in this case, we are dealing with events either for script-generated elements or for a limited time only.

Anonymous’s picture

Status: Fixed » Closed (fixed)