Closed Bug 404320 Opened 12 years ago Closed 12 years ago

FormatBlock doesn't work correctly will break existing Midas implemenations

Categories

(Core :: Editor, defect, P3, major)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta2

People

(Reporter: spam, Assigned: peterv)

References

()

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9

The Midas FormatBlock command doesn't work in the current Firefox 3 branch. I attached a test case that works perfectly fine in Firefox 2 but fails in the current implementation. The FormatBlock command is also not compatible with other browsers like IE, Safari, Opera.

Check this bug for details on the compatibility:
https://bugzilla.mozilla.org/show_bug.cgi?id=316544

Reproducible: Always

Steps to Reproduce:
1. Open attached test case in latest nightly build.
2. Place cursor/caret inside on of the editable elements.
3. Click one of the formatblock links.
4. Observe that the block doesn't get formatted correctly.
Actual Results:  
The code remains the same.

Expected Results:  
One of the elements that the caret was placed in should have been converted into a div element.
A follow up, it seems that the current implementation can only handle h1-h6,p,pre,address where the previous implementation could handle most of the block elements like div, code, dt, dl, samp etc.

It seems to be that it checks the format against the gMidasParamTable list. This should be extended with all block element.

http://lxr.mozilla.org/seamonkey/source/content/html/document/src/nsHTMLDocument.cpp#4228

This bug breaks existing editors like TinyMCE of FCKEditor those editors are used in the most popular CMS and Blog systems out there. So I think this is at least a major bug.
Severity: normal → major
Component: General → Editor
Keywords: regression
OS: Windows Vista → All
Product: Firefox → Core
QA Contact: general → editor
Hardware: PC → All
Version: unspecified → Trunk
I can reproduce this, but I'm not sure if it's intended since I don't know enough about Midas.

Moving to Core and requesting blocking, but not confirming.
Flags: blocking1.9?
peterv: can we get your weigh-in here?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch v1 (obsolete) — Splinter Review
Yeah, we started being more restrictive (FF2 accepted any tag name), but we're missing some tag names that IE and Safari accept. Note that HTML5 doesn't allow <div> at all, so we'll be incompatible with that.
Anyway, trivial to fix. I'll attach a mochitest too.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Attachment #289977 - Flags: superreview?
Attachment #289977 - Flags: review?
Attachment #289977 - Attachment is patch: true
Attachment #289977 - Attachment mime type: application/octet-stream → text/plain
Attachment #289977 - Flags: superreview?(jst)
Attachment #289977 - Flags: superreview?
Attachment #289977 - Flags: review?(jst)
Attachment #289977 - Flags: review?
Attached patch v1Splinter Review
Attachment #289977 - Attachment is obsolete: true
Attachment #289978 - Flags: superreview?(jst)
Attachment #289978 - Flags: review?(jst)
Attachment #289977 - Flags: superreview?(jst)
Attachment #289977 - Flags: review?(jst)
Attachment #289978 - Flags: superreview?(jst)
Attachment #289978 - Flags: superreview+
Attachment #289978 - Flags: review?(jst)
Attachment #289978 - Flags: review+
Attached patch MochitestSplinter Review
Comment on attachment 289978 [details] [diff] [review]
v1

Simple fix to make our designMode/contentEditable editing API more compatible with IE and Safari.
Attachment #289978 - Flags: approval1.9?
Comment on attachment 289978 [details] [diff] [review]
v1

a=beltzner for drivers
Attachment #289978 - Flags: approval1.9? → approval1.9+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: blocking1.9? → in-testsuite+
Priority: -- → P3
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M10
Depends on: 573527
You need to log in before you can comment on or make changes to this bug.