Opened 10 years ago

Closed 10 years ago

#908 closed defect (fixed)

Prompts broken in IE7

Reported by: guest Owned by: gogo
Priority: normal Milestone: Version 1.0
Component: Plugin_FormOperations Version: trunk
Severity: trivial Keywords: ie7 prompt image table extended forms
Cc:

Description

Due to a "feature" of IE7, the cell merge is broken in IE7, as are a few other bits in Image Library / Extended File Manager / Forms due to the use of the javascript "prompt" command.

For more details on the issue, see: http://www.hunlock.com/blogs/Working_around_IE7s_prompt_bug,_er_feature


David G. Paul

Attachments (7)

table-operations.js (38.4 KB) - added by wymsy 10 years ago.
table-operations.js
merge_cells.html (1.4 KB) - added by wymsy 10 years ago.
popups/merge_cells.html
table-operations.2.js (39.7 KB) - added by ray 10 years ago.
table-operations.js
merge_cells.2.html (1.4 KB) - added by ray 10 years ago.
popups/merge_cells.html
table-operations.3.js (39.1 KB) - added by wymsy 10 years ago.
table-operations.js
merge_cells.3.html (1.5 KB) - added by wymsy 10 years ago.
popups/merge_cells.html
table-operations.4.js (40.3 KB) - added by ray 10 years ago.
table-operations.js

Download all attachments as: .zip

Change History (25)

comment:1 Changed 10 years ago by ray

Oh my... :(
This security stuff is hell from beginning to end

comment:2 Changed 10 years ago by gogo

  • Milestone set to Version 1.0

That has got to be the suckiest thing Microsoft has done in a long time. I guess there is not much that can be done except to work around, and it will have to be a big work around, not only can we not use prompt, but it will require structural changes to the code that uses prompt currently due to the fact we can't get the value prompted for synchronously.

Big suckage.

Unfortunatly, I see no alternative than to push this to 1.0 Milestone, we can't have IE7 not working properly with a 1.0 release :-(

comment:3 Changed 10 years ago by ray

  • Priority changed from normal to highest

comment:4 Changed 10 years ago by wymsy

I haven't had time to try this, but one way to address this in the cell merge case might be to use a dialog popup to replace both prompts at once, which would also actually be an improvement in the user interface.

comment:5 Changed 10 years ago by wymsy

I have rewritten the cell-merge in table-operations.js to:

  • replace the two prompts with a single dialog box
  • fix ticket:82 (merging an already-merged cell breaks the table)

I have tested this in FF and in IE6. The files are attached above. Before I commit this, would somebody please test it in IE7 and see if it also fixes ticket:947 and/or ticket:912 ?

Changed 10 years ago by wymsy

table-operations.js

Changed 10 years ago by wymsy

popups/merge_cells.html

Changed 10 years ago by ray

table-operations.js

Changed 10 years ago by ray

popups/merge_cells.html

comment:6 Changed 10 years ago by ray

Honestly, I wasn't able to merge just 2 cells in one row or col, because I couldn't figure out what combination of numbers I had to put in the two fields. When I put in 0 in, say, rows when I wanted to merge two cells in one row and no rows, I got errors. I think it's just a bit un-intuitive to ask for rows and cols at once.

In the version of the files I just attached, the popup only asks for one number and then the direction in which to merge.

Also, in Firefox when there's no selection of more than one cell, the popup is now produced.

I think this version definitely fixes #947 & #912

A problem here still occurs when merging cells that already are merged, but I think that's a different topic...

comment:7 Changed 10 years ago by wymsy

I don't think asking for columns and rows in the same popup is any less intiutive than the original two prompts, and with your version you can only merge cells in the same row or in the same column, but not both (e.g. merge a 2 x 3 block of cells.) I agree that the wording in the popup could be improved, something like

"Merge current cell with

_ cells to the right

_ cells down"

I like your idea to bring in the popup in FF when only one cell is selected.

My version includes a fix for merging cells that are already merged. This works well in IE. However, in FF it is possible to select an area that is not rectangular if there are merged cells within the area, which still results in a broken table. I have an idea how to get around that. I'll post it when I get it working.

comment:8 Changed 10 years ago by ray

Actually I don't think tat it occurs so often that a user wants to perform a merging action that involves rows and lines. However more important is of course that more complex configurations work at all, if at once or after another. Quite hard indeed.. i've got the feeling that this plugin is the most complex part of Xinha at all, and there's LOADS of room for improvement :)

Changed 10 years ago by wymsy

table-operations.js

Changed 10 years ago by wymsy

popups/merge_cells.html

comment:9 Changed 10 years ago by wymsy

I just attached a new version that gets around selection problems in FF involving already-merged cells. It uses colspans and rowspans of the selected cells to derive the number of columns and rows to merge, and then uses the same cellMerge() function as the IE popup to do the actual merge. It is still possible to make a selection that can't be merged (and you can specify the same action through the popup), and in these cases I catch the error, put up an alert("Invalid selection"), and bail out. But in virtually all rational examples it works correctly.

Changed 10 years ago by ray

table-operations.js

comment:10 Changed 10 years ago by ray

Cool, seems to work very well now :)

Two little things changed in table-operations.4.js:

  1. 644 : the sel.getRangeAt(0) has to be inside the non-IE block
  2. 467 & l. 469: if a user enters to high numbers, I find it nicer to just finish the loop instead of throwing an error

comment:11 Changed 10 years ago by ray

fixed for EFM & ImageManager

comment:12 Changed 10 years ago by ray

in rev [799]

comment:13 Changed 10 years ago by wymsy

Ray, thanks for your comments. I have committed this in [800]. I didn't include the changes to finish the loop, because more often than not that will result in a broken table. I think it's better to throw the error and give the user a chance to try again.

comment:14 Changed 10 years ago by ray

  • Resolution set to fixed
  • Status changed from new to closed

Seems to be all fixed by now, isn't it?

comment:15 Changed 10 years ago by wymsy

  • Component changed from Plugin_TableOperations to Plugin_FormOperations
  • Resolution fixed deleted
  • Status changed from closed to reopened

There is still one more prompt(), in form-operations.js.

comment:16 Changed 10 years ago by ray

  • Priority changed from highest to normal
  • Severity changed from normal to trivial

Btw, funny MS has dropped this absurdity and prompts work as they should again.

comment:17 Changed 10 years ago by wymsy

Hmm, that appears to be the case! So, shall we close this again and move on, or do you want to undo rev 799 first?

comment:18 Changed 10 years ago by ray

  • Resolution set to fixed
  • Status changed from reopened to closed

Well, that's was major piece of work and I don't fancy to take it back... They should be sued for the economical damage the inflict by making web developers do loads of extra work to compensate for their quirks

Note: See TracTickets for help on using tickets.