Opened 9 years ago

Last modified 7 years ago

#1283 new defect

New-dialogs version of TableOperations is incomplete

Reported by: nicholasbs Owned by:
Priority: normal Milestone: 0.97
Component: Plugins Version: trunk
Severity: normal Keywords:


I realized today that the version of the TableOperations plugin for new-dialogs is incomplete. Most notably, the settings from its various dialogs (e.g., Table Properties) do not get applied to the table.

I've looked into the code a bit and discovered at least one of the issues: the dialog is constructed dynamically in part using the InlineStyler? module. This means that when Xinha.Dialog.translateHtml creates its reverse ID index of all elements in the document, it does not actually get all of the form elements. This in turn means that dialog.hide() does not return values for some of the input elements, making it impossible for TableOperations to properly update the attributes.

Off the top of my head, I can think of a couple of ways to fix this:

  1. Hard-code the dialog HTML instead of dynamically generating it
  1. Add a method that updates Xinha.Dialog's reverse ID lookup index

I'm initially in favor of doing (2) but not strongly so, and I admit I haven't spent a lot of time thinking about this/looking at the code.

Thoughts? Ray, I believe you did the initial work on this plugin a while ago, how were you planning on tackling this?

Change History (4)

comment:1 Changed 9 years ago by ray

Oh shit I didn't realize this. seems like I commited this stuff before it was even ready. sorry. No opinion either, at the moment

comment:2 Changed 9 years ago by nicholasbs

Changeset r1075 is a first pass at finishing up TableOperations, but there are still some remaining issues. Most of these have to do with setting cell/row border attributes.

In particular, setting cell or row borders simply doesn't work in most cases. In FF, changes are only shown after clicking view source and then going back to WYSIWYG mode. In Safari, they never show up. The FF issue seems to stem from a redraw bug (Ray, from the comments in the code it looks like you may have been fighting with this before?). I'm not sure about the Safari bug, but it might be an issue of CSS styling on the individual TD elements overriding it, though I haven't spent a lot of time troubleshooting this.

Note that setting the border properties on the table as a whole works, and setting other properties, e.g., background color or text align, on a row or cell basis does work.

Also, I went with option (2) in that I added a function to Xinha.Dialog that takes an ID, adds it to the reverse lookup table, and then returns the ID which it maps to. This function should be used any time you dynamically add an element that needs an id/name to a dialog (particularly if you want its values to show up in the param object returned by Dialog.hide()).

comment:3 Changed 9 years ago by nicholasbs

I took another look at this and think I mostly figured out what's going on:

  1. The Firefox issue is indeed a redraw bug. You can see this for yourself by setting a border on a row; by default, you won't see any changes. If you switch to source view mode and back, however, the border will show up properly. You can also use Firebug to append a junk text node to the tr, which will trigger a redraw and cause the border to appear (see The issue is slightly more complicated/worse with cells, since you have to force a redraw of multiple elements.
  1. The Safari bug is actually a separate issue in which borders don't show up when the collapse borders option is not set, which was the default case for Safari due to a separate bug, which I've tickted as #1386.

(2) should be fixed when #1386 is fixed, but (1) is a bit more of a pain. I haven't yet found a good/non-hacky solution, but will poke at it some more when I get the chance (hopefully in the next few days) and see if I can come up with something.

In the meantime, anyone else have any ideas?

comment:4 Changed 7 years ago by gogo

  • Milestone changed from 0.96 to 0.97
Note: See TracTickets for help on using tickets.