Created attachment 547314 [details] testcase See testcase. Steps to reproduce: - Long tap on the text inside the iframe. Notice how you're not getting to see any selection handles (that's bug nr. 1) - Tap on the url bar, the awesome bar opens. - Press the Android 'back' button Notice how you're not able to get back to the web page in question, you're staying in the awesome bar.
The second problem (not being able to use the Android back button) is very annoying, so I'm asking tracking-firefox 7? for that.
If this is really two bugs, we should file two bugs
You're right, I have now filed bug 673166 for the second problem. I suspect they might be connected problems, however.
Same issue? http://www.browserscope.org/jskb/test -> Run tests - unable to select any of the text content
Indeed, those results are inside an iframe, so that is the same issue.
Created attachment 548985 [details] [diff] [review] WIP This starts to fix things. Needs more work to position handles correctly and fix the range sent to the parent process.
Created attachment 548996 [details] [diff] [review] Patch v1
Comment on attachment 548996 [details] [diff] [review] Patch v1 >+ // if this is an iframe, dig down to find the document that was clicked >+ var aX = json.x; >+ var aY = json.y; var -> let >+ let elem = utils.elementFromPoint(aX, aY, true, false); >+ while (elem && (elem instanceof HTMLIFrameElement || elem instanceof HTMLFrameElement)) { >+ // adjust client coordinates' origin to be top left of iframe viewport >+ let rect = elem.getBoundingClientRect(); >+ scrollOffset = ContentScroll.getScrollOffset(elem.ownerDocument.defaultView); >+ offsetX += rect.left; >+ aX -= rect.left; >+ >+ offsetY += rect.top + scrollOffset.y; >+ aY -= rect.top + scrollOffset.y; >+ utils = elem.contentDocument.defaultView.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); >+ elem = utils.elementFromPoint(aX, aY, true, false); >+ } No easy way to reuse elementFromPoint function at the top of the script I assume. It is using "getClosest" stuff, which probably isn't good for this >+ let contentWindow = elem.ownerDocument.defaultView; >+ let currentDocShell = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor). >+ getInterface(Ci.nsIWebNavigation). >+ QueryInterface(Ci.nsIDocShell); No wrap needed here >+ this.contentWindow = contentWindow; Can we assign this right away and drop the extra local variable? > case "Browser:SelectionEnd": { > try { > // The selection might already be gone >- content.getSelection().collapseToStart(); >+ this.contentWindow.getSelection().collapseToStart(); >+ this.contentWindow = null; We could use an "if (this.contentWindow)" but I guess we are in a try/catch so it should be fine > // Cache the selected text since the selection might be gone by the time we get the "end" message >- this.selectedText = content.getSelection().toString().trim(); >+ this.selectedText = this.contentWindow.getSelection().toString().trim(); if we don't have a contentWindow things would be bad, but we should probably return early from the function itself, not just here. Try to make sure we check this.contentWindow a bit more defensively Nicely done
inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/5a0b3bee1f8a
http://hg.mozilla.org/mozilla-central/rev/5a0b3bee1f8a
Verified fixed, using today's build on the LG Optimus Black. The issue mentioned in comment 4 doesn't seem to be fixed, so I guess a new bug should be filed for that.
It turns out the issue mentioned in comment 4 now suffers from bug 674531.
Comment on attachment 548996 [details] [diff] [review] Patch v1 mobile only and it helps stabilize the text selection feature
Comment on attachment 548996 [details] [diff] [review] Patch v1 Approved for releases/mozilla-aurora. Please land ASAP.
http://hg.mozilla.org/releases/mozilla-aurora/rev/9c7d60e7020b
Verified Fixed on Aurora Mozilla/5.0 (Android; Linux armv7l; rv:7.0a2) Gecko/20110805 Firefox/7.0a2 Fennec/7.0a2
or
