we should fix this soon; not specific to JS handlers (adding an id also fails) -->cmanske
I can't reproduce this in recent builds. Please test again.
spam composer change
I'm still seeing this bug. You must make sure you click on advanced edit after putting in the # for the link. If you click ok after adding the # and then go back to link properties and the advanced edit it won't work. This bug has been reproduced on the 20010910 build on Windows 9x pc's and on a Mac.
The code for setting attributes via advanced edit dialog is the same for all dialogs: Image, H.Line, Table, etc. Do you see the problem for any of those elements? Is it only specific attributes or JS events that aren't retained? Latter definitely shouldn't happen. Note that I'm using WinNT, but I can't see how platform should affect this.
Still can't reproduce this. Do you see the same problem in any other dialog (e.g., Image, HLink, Table, etc.)
I am able to reproduce this on build 2001091703 both on Win2k and Mac OS 9.1. This problem does not appear to happen with Image, table, HLine, or Anchor. One thing to note, is that you have to click on Advanced Edit when initially setting up the link, otherwise, it works fine.
I still can't reproduce this!
I see this following the steps provided initially. Make sure that the link isn't created before you go into the advanced edit dialog. All you need to add in the advanced edit dialog is id="foo" Do we have similar problems with other elements (images, anchors)? I think we fixed a similar bug with table insertion already.
This can happen when setting any kind of attribute. If you use any tab in the advanced editor to add attributes they won't be applied on creation. I tried this with anchors, images, and tables and they all work as they should. The H.Line just gets put in without giving a prompt to set it up so there is not a problem there. Also, if you make a link from nothing (i.e. not highlighting anything first) , then it will work fine. If you click the link button and it asks for you to type in the "Link Text" then to type in the "Link Location" the advanced edit settings will be applied.
Ok, I finally get it (sorry!), but it's a complete mystery why it's not working!
Ok, I'm not going crazy. All attributes except HREF are ignored by nsHTMLEditor::InsertLinkAroundSelection()! This comment is in that method: //TODO: Enumerate through other properties of the anchor tag // and set those as well. // Optimization: Modify SetTextProperty to set all attributes at once?
I thought I could optimize by finding the new anchor node created and use nsEditor::CloneAttributes, but if the selection spans across different elements, SetInlineProperty() will create > 1 anchor element! So it's better to just let SetInlineProperty() find existing anchors after the first attribute is set.
Comment on attachment 51123 [details] [diff] [review] Fix: Set all attributes on the anchor element * the nsAutoString declarations inside the loop should be moved out (name, value) * fix this line to have a space after "if": if(attrNode) * you left in this comment: // Optimization: Modify SetTextProperty to set all attributes at once? but it doesn't make sense to me since I don't see "SetTextProperty" anywhere Unfortunately this fix is going to massively collide with the work I did yesterday on a different bug in this same method. I'll have to back out what I did and wait for your fix and then start my stuff over. :-/ Who would have thought?! (given our very different bugs)
given all of the possible early return (error) situations in SetInlineProperty, shouldn't we check a return value for that call?
Created attachment 51520 [details] [diff] [review] Updated patch: Check return value from SetInlineProperty()
Comment on attachment 51520 [details] [diff] [review] Updated patch: Check return value from SetInlineProperty() r=brade
Taking this off the "fixed" radar. SetInlineProperty is buggy and we need to address those issues first.
I think attrMap->Item() addrefs the node it returns so there might be a leak here. + nsIDOMNode* attrNode; + res = attrMap->Item(i, &attrNode);
Comment on attachment 54504 [details] [diff] [review] Updated patch. email@example.com Are the bugs with SetInlinePropery that you mentioned above, still a problem?
Comment on attachment 54504 [details] [diff] [review] Updated patch. r=jfrancis