Closed
Bug 838547
Opened 12 years ago
Closed 12 years ago
Appending an iframe makes input field lose focus.
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(blocking-b2g:leo+, firefox22 fixed, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)
People
(Reporter: jj.evelyn, Assigned: kanru)
References
Details
(Whiteboard: [LeoVB+])
Attachments
(2 files, 3 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
kanru
:
review+
|
Details | Diff | Splinter Review |
I'm trying to create an iframe for loading 3rd party keyboard when an input field gets focus. However, the the input field always lose its focus once the iframe is appended to the DOM tree (so I get a 'blur' event immediately).
Reporter | ||
Comment 1•12 years ago
|
||
Description: The onfocus callback of the first input field will append an iframe, while the second input field will append a button. Result: I tested this case on three different environments: 1. Firefox desktop: both input fields won't lose their focus 2. Browser App on Firefox OS: both input fields won't lose their focus 3. an app on Firefox OS: the first input field will lose its focus once the iframe is appended. So I suspect the problem is related to frame script of Firefox OS.
Assignee | ||
Comment 2•12 years ago
|
||
In bug 820057 we added the pagehide event listener but it receives pagehide events when an iframe is appended. Add the event listener to the content window should let us receive only the event we are interested. Perhaps someone more familiar with frame scripts could tell why we are receiving extra pagehide event from the global object.
Assignee: nobody → kchen
Status: NEW → ASSIGNED
Attachment #715085 -
Flags: review?(ttaubert)
Attachment #715085 -
Flags: review?(margaret.leibovic)
Comment 3•12 years ago
|
||
Comment on attachment 715085 [details] [diff] [review] Only listen to pagehide event of the content window I unfortunately don't have a current build to test and investigate this. I hope Margaret still does? I also think David might be a good candidate as well, he knows about forms.js, too :)
Attachment #715085 -
Flags: review?(ttaubert) → review?(dflanagan)
Reporter | ||
Updated•12 years ago
|
Blocks: 3rd-party-keyboard
Comment 4•12 years ago
|
||
I also don't have a current build to test, and it's been a while since I've thought about this code. My first question would be why we'd only change this one event listener to be added to the content window. Couldn't there be similar event bubbling issues that affect the other listeners? Looking at the XUL fennec implementation, there's a different check before deciding whether or not to hide the keyboard: http://mxr.mozilla.org/mozilla-central/source/mobile/xul/chrome/content/forms.js#316 Maybe we need to do something like that in b2g. I also noticed that the event listener in XUL fennec doesn't initiate capture, but the listener in b2g does. I don't know that this would cause the issue you're seeing here, but it would be nice to know the reasoning behind that difference. Maybe Tim knows.
Comment 5•12 years ago
|
||
Comment on attachment 715085 [details] [diff] [review] Only listen to pagehide event of the content window Review of attachment 715085 [details] [diff] [review]: ----------------------------------------------------------------- I'm not good with chrome code.... Help me understand this patch so I can review it. IIRC, forms.js gets injected into every running app, correct? What is the practical difference in forms.js between addEventListener() and content.addEventListener()? content is the window object, right? What is the global object of forms.js? What kind of pagehide event is being fired on that global object rather than bubbling up from content? Basically, I just can't see why this works. Instead of registering this one listener on a different object than all the others, could we instead check the target field of the event object when we handle the event? I'm not giving r+ or r- now, because I don't yet understand the patch.
Comment 6•12 years ago
|
||
Tim, It looks like you added the pagehide event handler as part of the fix for bug 820057. Do you recall what it was needed for? What case is there where we'd get a pagehide but not get a blur or submit before the pagehide? I'm guess I'm asking whether it might be possible to get away with not listening for pagehide at all. Also, does anyone on the bug understand why a pagehide event is being generated when an iframe is added?
Flags: needinfo?(ttaubert)
Comment 7•12 years ago
|
||
(In reply to David Flanagan [:djf] from comment #6) > Also, does anyone on the bug understand why a pagehide event is being > generated when an iframe is added? I agree we should try to find an answer to this question. Does that happen on desktop Firefox?
Comment 8•12 years ago
|
||
Comment on attachment 715085 [details] [diff] [review] Only listen to pagehide event of the content window Taking myself off the review since as noted above I don't understand that patch. I'm happy to try again with some explanation. But I don't want it to look like this is a review I'm still working on.
Attachment #715085 -
Flags: review?(dflanagan)
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 715085 [details] [diff] [review] Only listen to pagehide event of the content window Remove all reviews until I found the root cause.
Attachment #715085 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 10•12 years ago
|
||
The extra pagehide events are chrome events from subframes. This patch checks if event target is the root document.
Attachment #715085 -
Attachment is obsolete: true
Attachment #721628 -
Flags: review?(dflanagan)
Attachment #721628 -
Flags: review?(bugs)
Comment 11•12 years ago
|
||
Comment on attachment 721628 [details] [diff] [review] We are only interested to the pagehide event from the root document Don't know why you have 'target instanceof HTMLDocument' check. What if SVGDocument is loaded? Why you filter out also blur and submit?
Attachment #721628 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 12•12 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #11) > Comment on attachment 721628 [details] [diff] [review] > We are only interested to the pagehide event from the root document > > Don't know why you have 'target instanceof HTMLDocument' check. > What if SVGDocument is loaded? > Why you filter out also blur and submit? The check is there so that we don't filter out blur and submit event. Maybe I should instead move pagehide to its own block.
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #721628 -
Attachment is obsolete: true
Attachment #721628 -
Flags: review?(dflanagan)
Attachment #721637 -
Flags: review?(bugs)
Comment 14•12 years ago
|
||
Comment on attachment 721637 [details] [diff] [review] We are only interested to the pagehide event from the root document v2 Just drop + if (this.focusedElement) + this.hideKeyboard(); + break; so the execution continues to the blur/submit case.
Attachment #721637 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #721637 -
Attachment is obsolete: true
Assignee | ||
Comment 16•12 years ago
|
||
Seems inbound will be closed for a while. Add checkin-needed so I don't forget this.
Keywords: checkin-needed
Comment 17•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc9cdf372502
Keywords: checkin-needed
Comment 18•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fc9cdf372502
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)
Updated•12 years ago
|
Flags: needinfo?(ttaubert)
Comment 19•11 years ago
|
||
Going to nominate this as leo+, because it blocks a leo+ issue, Bug 890441.
blocking-b2g: --- → leo?
Comment 21•11 years ago
|
||
Cool! Leo+ so we need to uplift this right? Kan-Ru could you help us on this? I will take a look for ensuring that the other bug relate is working as expected once we have this uplifted. Thanks!
Flags: needinfo?(kchen)
Assignee | ||
Comment 22•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/8c0127cc7be3
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-b2g-v1.1hd:
--- → affected
Flags: needinfo?(kchen)
Comment 23•11 years ago
|
||
Thanks for the quick uplift! Gracias ;)
Comment 24•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/8c0127cc7be3
status-firefox22:
--- → fixed
Updated•11 years ago
|
Whiteboard: [LeoVB+]
Comment 25•11 years ago
|
||
Verified fixed on Leo 1.1 mozilla RIL. The Maps app is no longer present on the homescreen but can be downloaded from the Marketplace. Build ID: 20130723070209 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/68fb0a2e0114 Gaia: ffe25bfdf0e71c820ca710cb61fb8306564a8f4e Platform Version: 18.1
You need to log in
before you can comment on or make changes to this bug.
Description
•