Closed Bug 69669 Opened 24 years ago Closed 24 years ago

focus doesn't go to editor field

Categories

(Core :: DOM: Editor, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla0.9

People

(Reporter: tracy, Assigned: morse)

References

Details

(Keywords: smoketest)

seen on commercial builds: windows 2001-02-21-06-mtrunk linux 2001-02-21-06-mtrunk mac 2001-02-21-04-trunk -open composer or attempt to edit page the focus doesn't go to the editor field, thus nothing can be accomplished with composer this is also affecting compose mail...attempts to create new message get an error saying Reference Error: HideImage is not defined
Keywords: smoketest
It appears that this problem comes from the remove of "HideImage()" (JS function) but not removing it from the entire tree. (Or moving it and not including the new file in the editor files?) Reassign to Steve Morse, cc Charley and others
Assignee: beppe → morse
confirming in mail composition... This is a severe blocker !
I'm looking into it right now.
Status: NEW → ASSIGNED
This is a bit baffling. I did remove HideImage from the common code so indeed I would think that I should get a failure when the editor was calling it. But in my own build, I don't get the failure and the hideimage call in the editor is succeeding. Can't explain that yet. Still investigating. Although I can't yet reproduce the problem (and don't know why), I agree with brade's suggestion (in e-mail to the hook) that the temporary fix is to remove the call to HideImage in editor.js. Question is, why was that call added in the first place. The comment (made by cmanske on Aug 9 2000) is "Remove a privacy menu that causes problem". That menu was apparently the image manager. Charlie, do you recall what problem that was causing and what would be the downside of not removing that menu?
Since this is working in my local tree, I need to pull and build a fresh tree to find out what is going wrong. This will take me two hours and I just started the pull now. Suggest that we simply comment out the HideImage call (as brade suggested) so this is not a tree blocker for today. By tomorrow we should have the correct fix.
I'm ok with checking in the workaround as long as we get the real fix later today. can someone check in that workaround so we can reduce the severity to critical and get this off the blocker radar?
Morse: Why did you remove the function HideImage()? Are you sure these menus are supposed to work when in Composer? Don't we still need to suppress those menuitems? Jon: Which temporary fix would you like: Comment out the "HideImage()" call in editor is simplest, I guess.
comment out the hide image is what I was thinking.
I didn't remove HideImage, I moved it to overlays in extensions/cookie.
Well, the real questions are: 1) Why was HideImage(), a cookie manager function it seems, called from editor in the first place? 2) Is it still needed? I just tried replying to an email using 2001022105 win32, got the "Reference Error: HideImage is not defined" alert, and don't have a mail compose window. Not good.
I checked in the fix granrose requested; downgrading to critical per granrose's comments above
Severity: blocker → critical
Target Milestone: --- → mozilla0.9
Blocks: 68244
As we ponder on what the correct fix should be, let me just comment on what the HideImage routine does. In the mozilla build, there are four managers -- cookie, image, forms, and password. We decided that we didn't want the image manager in the commercial build. So the HideImage routine removes the image manager from the tasks menu.
Steve--so what is the purpose for HideImage in Composer? (and mail compose) Charley alludes to some problems with some of those menu items in Composer. Have those issues been resolved so that they now work in Composer?
I wasn't even aware that it was being called from composer. The purpose would be to remove the image-manager entry from the task menu when composer was running. I have no idea what problem charlie had run into. Best to let him comment on that.
*** Bug 69683 has been marked as a duplicate of this bug. ***
Steve described exactly why we use HideImage(): "The purpose would be to remove the image-manager entry from the task menu when composer was running". Thus it must be called from Composer, no? So the bug is that you moved it to a location we can't access. But looking at where you moved it, you are looking at the value of a pref, not whether or not we are an editor. All I know about why we used HideImage() is because that menu items caused problems in the editor. So I'm tempted to replace the "HideImage()" call with what it does: element = document.getElementById("image"); element.setAttribute("disabled","true" ); and be done with it. Note that you really don't need to do: element.setAttribute("style","display: none;" ); Setting "display: none" does exactly that.
> But looking at where you moved it, you are looking at the value of a pref, > not whether or not we are an editor. That's true. The pref allows us to remove the menu item when in the commercial browser and not remove it when in the mozilla browser. > All I know about why we used HideImage() is because that menu items caused > problems in the editor. Do you recall what those problems were? > So I'm tempted to replace the "HideImage()" call with what it does: Be careful here. The reason I did the restructuring last night was because all these menu items refer to code that is in the extensions directory and the core browser is not supposed to know anything about that directory. Since the editor is in the core browser, you shouldn't know about the image menu. The key question is, what's the downside to keeping the image menu when in composer? If none, then let's do nothing more than removing the call to HideImage (which was done already). If there is a downside, then perhaps the code in the extensions directory needs to explicitly test for composer and supress the menu item if composer is running.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
working fine on commercial builds: windows 2001-02-22-10-mtrunk linux 2001-02-22-06-mtrunk mac 2001-02-22-04-trunk
verified in 2/22 build.
Status: RESOLVED → VERIFIED
*** Bug 69937 has been marked as a duplicate of this bug. ***
*** Bug 70137 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.