Closed Bug 127368 Opened 23 years ago Closed 23 years ago

Can't type in textfields in pages with an iframe and body with marginheight and marginwidth=0

Categories

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

Product:
Core
Component:
DOM: Editor
Platform:
x86
Windows 2000
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.9

People

(Reporter: stephend, Assigned: kinmoz)

References

(
URL
)

Details

(Keywords: regression, Whiteboard: EDITORBASE+)

Attachments

(3 files)

Reduced home.netscape.com Test Case
135 bytes, text/html
Details
Patch Rev 1
868 bytes, patch
roc
: review+
sfraser_bugs
: superreview+
shaver
: approval+
Details | Diff | Splinter Review
Proposed alternative patch for 1.0
897 bytes, patch
Details | Diff | Splinter Review
Build ID: 2002-02-22-03, Windows 2000. Summary: Can't type in textfields using document.write(searchform) (I suck at making testcases, forgive me) Steps to Reproduce: 1. Visit http://home.netscape.com 2. Try to type in any form textfield. Expected Results: Can type in textfields. Actual Results: No textfield on this page works. (other at my.netscape.com, etc. work)
This regression occured sometime between 2002-02-19 and 2002-02-20 trunk build. Depending on when the source was pulled in Mozilla build, you might see this occurring on the 2002-02-20 build instead. I see this problem with 2002-02-19 commercial build.
Kat when you say you see the problem with a 2002-02-19 Commercial build, do you mean one that you got off sweetlou or one you built yourself? If from sweetlou, what's the build id, that could give us a clue as to how to narrow down time? There were a ton of checkins between the 02/18 and 02/20 due to the mozilla0.9.9 deadline.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.9
This bug occurs at http://www.lycos.com on build Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.8+) Gecko/20020222 To replicate: -Go to http://www.lycos.com -Try to type in search term Result: nothing happens Click Lycos Mail tab, and you can see that you can type into those fields as expected. Replicatability: always Works for me in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.4) Gecko/20011128
URL: http://home.netscape.com → http://home.netscape.com, http://www....
confirming. I see this problem on today's 2/25 trunk build.
*** Bug 127601 has been marked as a duplicate of this bug. ***
FYI, I've been poking around looking at this, and it seems that the presShell, presContext, and EventStateManager used to process the click that sets focus in the text widget, is different from the one that is used to process the keypress event generated when typing into the text widget ... the result is that the event state manager used to process the keypress, does not have an mCurrentFocusContent set, so keypresses are being sent to the root <HTML> node of the document. I'm wondering if the document.write() triggers frames to be recreated ... perhaps we are leaking a bunch of frames resulting in 2 presShells/editors being active.
Here's the reduced test case based on the home.netscape.com page. Note that there is no document.write() necessary to reproduce this bug. Some things I've noticed: 1. Both the home.netscape.com, and lycos.com page specify marginwidth=0 and marginheight=0 on the body. If I remove one or both of them, or I make one of them non-zero, I can type in the textfield just fine. 2. If I remove the iframe, or set frameborder="1" on the iframe, I can type just fine.
Whiteboard: EDITORBASE
Btw, the lycos homepage problem boils down to the exact same body/iframe interaction problem.
Ok, I have 2 linux debug builds, one from around 02/19 (9am pull), and one from 02/20 (9am pull) ... and the problem only shows up in my build from the 20th. Unfortunately there were a ton of checkins in that 24 hour period due to the mozilla0.9.9 freeze. Here are the checkins between 02/19 8am and the opening of the tree after verification on the 09/20. http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=02%2F19%2F2002+08%3A00%3A00&maxdate=02%2F20%2F2002+15%3A30%3A00&cvsroot=%2Fcvsroot
Blocks: 122050
Changing summary from: Can't type in textfields using document.write(searchform) to: Can't type in textfields in pages with an iframe and body with marginheight and marginwidth=0 since it has nothing to do with document.write().
Summary: Can't type in textfields using document.write(searchform) → Can't type in textfields in pages with an iframe and body with marginheight and marginwidth=0
*** Bug 127711 has been marked as a duplicate of this bug. ***
I just backed out roc+moz@cs.cmu.edu's view manager changes in my tree and it fixes the problem. Now that I have a clue as to what change is causing the problem, I'm putting the changes back in to see why.
Ok, it looks like it's this change in particular: roc+%cs.cmu.edu: *************** roc+%cs.cmu.edu: *** 2005,2011 **** roc+%cs.cmu.edu: aEvent->point.x -= x; roc+%cs.cmu.edu: aEvent->point.y -= y; roc+%cs.cmu.edu: roc+%cs.cmu.edu: ! obs->HandleEvent(v, aEvent, &status, i == targetViews.Count() - 1, handled); roc+%cs.cmu.edu: roc+%cs.cmu.edu: aEvent->point.x += x; roc+%cs.cmu.edu: aEvent->point.y += y; roc+%cs.cmu.edu: --- 2035,2052 ---- roc+%cs.cmu.edu: aEvent->point.x -= x; roc+%cs.cmu.edu: aEvent->point.y -= y; roc+%cs.cmu.edu: roc+%cs.cmu.edu: ! nsViewManager* vVM = v->GetViewManager(); roc+%cs.cmu.edu: ! if (vVM == this) { roc+%cs.cmu.edu: ! if (nsnull != obs) { roc+%cs.cmu.edu: ! obs->HandleEvent(v, aEvent, &status, i == targetViews.Count() - 1, handled); roc+%cs.cmu.edu: ! } roc+%cs.cmu.edu: ! } else { roc+%cs.cmu.edu: ! nsIViewObserver* vobs = nsnull; roc+%cs.cmu.edu: ! vVM->GetViewObserver(vobs); roc+%cs.cmu.edu: ! if (nsnull != vobs) { roc+%cs.cmu.edu: ! vobs->HandleEvent(v, aEvent, &status, i == targetViews.Count() - 1, handled); roc+%cs.cmu.edu: ! } roc+%cs.cmu.edu: ! } roc+%cs.cmu.edu: roc+%cs.cmu.edu: aEvent->point.x += x; roc+%cs.cmu.edu: aEvent->point.y += y; Do we really want to send events to a view with it's own widget and a different ViewManager? That seems a bit dangerous, since the intended view/widget won't get the event if it is handled by this "other" view.
Btw, the above diff is for mozilla/view/src/nsViewManager.cpp.
Btw, the change was in nsViewManager::HandleEvent() not DispatchEvent() ... the darn '{' is up next to the name of the function so vi screws up when going to the start of a function. :-)
Ok, I see exactly why this is all happening. HandleEvent() is relying on the point inside of the event to decide whether or not to dispatch the event to a given view. KeyEvents *always* have a (0,0) point in their event, and since the body has a marginwidth and marginheight of zero, the iframe in question has it's view/widget positioned at (0,0) in it's containing view/widget. So I think the fix here is to not forward KeyEvents past ViewManager boundaries, since they should always go to the widget that has focus.
Attached patch Patch Rev 1 — — Splinter Review
This patch prevents the forwarding of key events to view's under other ViewManagers. Can I get a review from kmcclusk and a super review from roc+moz@cs.cmu.edu?
Whiteboard: EDITORBASE → EDITORBASE, FIX IN HAND, need r= and sr=
Keywords: nsbeta1 → nsbeta1+
I like this patch better. It just doesn't make any sense to use coordinate-based dispatch to resolve key events (except maybe for IMEs? I don't know anything about IMEs). It's a bit risky though so I prefer kin's patch for 0.9.9. Maybe we can consider this alternative patch for 1.0.
In case it's not obvious: kin's patch is safer because it changes behavior in fewer cases. Although it doesn't make sense to look for views containing (0,0) for key dispatch, I thought maybe we mistakenly rely on it somewhere so I didn't change the behavior. Kin's patch basically restores exactly the behavior that we used to have before my IFRAME stuff. My proposed patch fixes the real problem by always dispatching key events directly to the focused view. I suggest we take kin's for 0.9.9 and mine for 1.0.
Comment on attachment 71719 [details] [diff] [review] Patch Rev 1 sr=sfraser, but roc+moz should test IME and scroll wheel to make sure that they are not affected.
Attachment #71719 - Flags: superreview+
Whiteboard: EDITORBASE, FIX IN HAND, need r= and sr= → EDITORBASE, FIX IN HAND, need a=
Love to, but I don't have IME or scroll wheel support on this system. Now that I think about it, if IME or anything else currently depends on key events being dispatched to (0,0), then they're just plain broken and must be failing horribly in lots of situations. Anyway, that's a question for patch 71778. kin's patch 71364 won't affect IME or scroll wheel (except possibly by fixing them for the pages mentioned in this bug).
I've a similar problem here : http://www.zonejeux.com (but only when you have already registered and you came back) The ads in the left (125x125) is an iframe. When typing into pseudo / password fields, this is a weird display of this iframe... I figure out why. I guess it's a bug !
Comment on attachment 71719 [details] [diff] [review] Patch Rev 1 a=shaver for 0.9.9.
Attachment #71719 - Flags: approval+
Whiteboard: EDITORBASE, FIX IN HAND, need a= → EDITORBASE, FIX IN HAND, ready for checkin
Patch Rev 1 checked into TRUNK: mozilla/view/src/nsViewManager.cpp revision: 3.231
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: EDITORBASE, FIX IN HAND, ready for checkin → EDITORBASE
*** Bug 128442 has been marked as a duplicate of this bug. ***
Is there another bug to track the 1.0-version of the patch now when the bug is marked FIXED?
Ok, I opened bug 128478 (ViewManager: KeyEvents shouldn't use coordinate-based event dispatching) to track the proposed mozilla1.0 patch.
verified in 3/1 windows trunk build.
Status: RESOLVED → VERIFIED
Whiteboard: EDITORBASE → EDITORBASE+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: