60868 - Insert HTML button in Composer does not work with Classic and Ski Pilot themes.

Status: VERIFIED FIXED

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

Description

[hirata masakazu]

New page to edit.
Insert|HTML...
Type whatever valid HTML source.
Hit Insert.
Nothing happens.
Mtrunk 2000112004

Comment 1

[hirata masakazu]

Typo in the summary corrected.

Comment 2

[kinmoz]

I just verified that this is working in my 12/12/00 Win32 and Mac Trunk debug
builds. I was able to add images, tables, styled text, etc, all from the Insert
HTML dialog.

Marking fixed, and adding [e-mail] to Cc list.

Comment 3

[hirata masakazu]

I do not know what a trunk debug build is, but I have seen no change in this
problem in MTrunk 2000121514, 121508, etc.  If there is any other builds that
does not have this problem, fine.  I expect the fix to reach nightly builds.
Until then, I keep this bug open.

Comment 4

[kinmoz]

I'm just curious, what HTML are you inserting as your testcase?

Comment 5

[Charles Manske]

It works just fine in the NS 6 release and in my current mozilla trunk build.
You must supply exactly what you typed into the edit box for your testing.
What happens when you click on "Insert" button? Does dialog close? Do you see
anything in the document at all?
The only problem I see is that the multiline textfield appears to be "invisible"
because its background is the same color as the dialog. When you click into it,
you see a shaded border and you can type HTML source then. Initial focus should
be set automatically when dialog is opened, but that's a separate bug.

Comment 6

[hirata masakazu]

I typed the exact example in the Insert HTML window, for instance.
Whatever I type, hitting insert button does nothing.
No response whatsoever.  Dialog window just remains there.
This is still true in 2000121808 MTrunk build.

Comment 7

[Charles Manske]

Are you using a debug build? We have been seeing problems with getting the "value"
from a textfield (any textfield). That might be the problem.
Try this: Click on the toolbar "Table" icon and try to insert a table. Does it
not respond when you click on OK?

Comment 8

[hirata masakazu]

I only test nightly binaries.  Not the one with installer.
The only insert function broken is Insert HTML. Others (table, symbol, anchor
etc.) work fine.

Comment 9

[Charles Manske]

Well thats to bad because, unfortunately, I can't reproduce the problem you
report!

Comment 10

[hirata masakazu]

I nailed the problem.  I thought it was strange that you described a background
problem that I did not see at all.  That was the key.  I use Classic skin and
you use Modern skin.  When I changed to Modern skin, indeed Insert button worked.
Changed the summary to include this information.

Comment 11

[hirata masakazu]

BTW, a lot of Mac users prefer Classic skin because of its resemblance to
standard MacOS look.  Modern skin may look cool, but does not bode well in daily
usage, IMHO.

Comment 12

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Just fyi about other platforms:
I am running ns6, nightly builds and my own build on a win32 box. I have tested
the three basic themes, including classic. I can't reproduce your bug. The HTML
fragment I type is correctly inserted and rendered at insertion point.

Comment 13

[Simon Fraser [no longer active]]

The original bug here happens because of this code in dialogOverlay.js:

function doOKButton()
{
    var v = document.commandDispatcher.focusedElement;
			
    if (v && v.localName.toLowerCase() == 'textarea')
      return;

We hit that 'textarea' return case. This code is obviously wrong (my guess is 
that it's there to allow the return KEY to be entered in textareas in dialogs).
Why this only happens in Classic is probably focus related.

Comment 14

[Charles Manske]

Thank you, Simon!

Comment 15

[Charles Manske]

Simon: Unfortunately, that is not the problem. We are getting to our own
"doOKFunction" just fine. Some dumps reveal that in doOKButton, the focused
element is the button, not the textarea, so this isn't the cause of the
get value failure.

Comment 16

[hirata masakazu]

 Just out of curiosity, I tested Sky Pilot.  It too shows this bug, and it,
neither, shows the background problem.  Probably not a big deal about the fact
itself.
 Changing Summary to include this information.

Comment 17

[Charles Manske]

Hirata: Are you still seeing this problem? I tried it yesterday on a Linux
build and could not reproduce the problem.

Comment 18

[hirata masakazu]

Yup. It is still here in 2001011104.
Has it ever been confirmed in Linux builds, BTW?
The button works fine in Modern skin just like before.

Comment 19

[Kathleen :Brade]

This is a mac-specific bug (apparently).  I see this on my Mac build from today.  
Reassign to myself.

Comment 20

[Kathleen :Brade]

The fix to bug #16339 seems to have broken this bug.
However, the keybindings in dialogOverlay.xul are no longer present so removing 
the extra JS in dialogOverlay.js would seem to be appropriate now.

Note:  Also broken (and presumably fixed with this fix) is the AddressBook New 
Card dialog and Bookmarks edit dialog.

patches coming in a little bit.

Comment 21

[Kathleen :Brade]

Attachment: patch for hyatt to review (remove code from dialogOverlay.js)

Comment 22

[Kathleen :Brade]

Attachment: patch file for Charley and Kin to review (cleanup)

Comment 23

[kinmoz]

sr=[e-mail] on most of the editor clean up diffs ... I'll defer 
reviewing the one change about including the 2 extra overlay files to cmanske 
since I'm not so  sure they are neccessary.

Comment 24

[Alec Flett]

sr=alecf on the first patch

Comment 25

[Kathleen :Brade]

fixed (although there is still some related cleanup to do in editor-specific 
files)

Comment 26

[sujay]

verified in 1/26 trunk build.