Ticket #617 (assigned Bug)

Opened 15 months ago

Last modified 8 months ago

Table: TD should use style to set borderColor

Reported by: geirhelge Owned by: martinkou
Priority: Normal Milestone:
Component: General Version: FCKeditor 2.4.3
Keywords: Review- Cc:

Description

When selecting borderColor for a TD it's set as an attribute. There's no borderColor attribute for the TD element (works in most browsers, but not valid). Style="bordercolor:#xxxxxx" should be used instead.

Attaching suggested fix

Attachments

fck_tablecell.html.patch (1.7 kB) - added by geirhelge 15 months ago.
Suggested fix
617.patch (2.1 kB) - added by martinkou 9 months ago.
Proposed patch for solving #617.
617_2.patch (9.3 kB) - added by martinkou 9 months ago.
Proposed fix for #617, version 2.

Change History

Changed 15 months ago by geirhelge

Suggested fix

  Changed 11 months ago by martinkou

  • owner set to martinkou
  • status changed from new to assigned
  • milestone set to FCKeditor 2.6

  Changed 11 months ago by martinkou

  • keywords HasPatch added

  Changed 10 months ago by fredck

Note on this: it must be backward compatible, looking for the color value in the borderColor attribute on load, and removing the attribute when setting the style on save.

  Changed 9 months ago by martinkou

The proposed patch does not work under Firefox, so I'm proposing another patch.

Changed 9 months ago by martinkou

Proposed patch for solving #617.

  Changed 9 months ago by martinkou

  • keywords Review? added; HasPatch removed

  Changed 9 months ago by alfonsoml

The problem from my point of view would be that if someone pasted content from another source, usually Word, (or manually edited the source), and that cell have a custom border like different colors for each border or width!=1px or style!=solid, then as soon as he edited the cell those properties would be lost.

I would rather have settings to also change the width and style of the border, and being able to set all the borders the same or each one independently.

But in fact I would rather have a style editor instead of putting everything in the main dialog for the cell (because people also want to customize color, background-color, background-image...)

  Changed 9 months ago by martinkou

  • keywords Review? removed

Another problem I just found is that different browsers are giving out vastly different HTML outputs for the border styles. Firefox gives the cleanest HTML output, while Safari attaches a huge assortment of CSS styles to the HTML output.

Changed 9 months ago by martinkou

Proposed fix for #617, version 2.

follow-up: ↓ 10   Changed 9 months ago by martinkou

  • keywords Review? added

I've added an updated patch for fixing this bug. The changes are moderately extensive for a seemingly simple bug.

The updated patch improves 2 things:

  1. If there's any already existing border styles in the table cell ( e.g. a dashed 2px border), the non-color border styles will not be destroyed.
  2. The HTML output of border styles is now the same across all browsers.

  Changed 8 months ago by martinkou

The v2 patch should have fixed #460 as well.

in reply to: ↑ 8   Changed 8 months ago by fredck

  • keywords Review- added; Review? removed

Replying to martinkou:

The changes are moderately extensive for a seemingly simple bug.

You are correct Martin, and the extension of the changes could bring more problems than benefits. I think we must review the strategy for this ticket.

To easily give a Review-, the changes on _GetMainXmlString should be moved to GetXHTML, together with all other string processing we are doing there. In any case, I would ask a hold in this ticket for better understanding over the current patch.

In any case, this ticket has been mistakenly targeted to the 2.6. I'm re-targeting it to the 2.7, which is related to the table support review.

  Changed 8 months ago by fredck

  • milestone changed from FCKeditor 2.6 to FCKeditor 2.7

  Changed 8 months ago by martinkou

#171 is also partially related to this ticket, in that part of the proposed patch can be used to fix #171.

Note: See TracTickets for help on using tickets.