593755 - Blur of <browser> within chrome process does not blur <element> within <browser> object

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Jeremias Bosch (:jbos)]

In Meego we want to synchronize the chrome and the content processes focusmanager. 

Which means:
   In case the <browser> object gets blured we want to blur the element within the <browser> and set the focus within the browser back to content.

Comment 1

[Jeremias Bosch (:jbos)]

Attachment: Version 1

This is a way to fix the issue.
It simply connect the blur and foxus event to the blur and focus IPDL Message. Blur in content.js gets extended with a clearFocus(content);

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 472334 [details] [diff] [review]
Version 1

>diff --git a/chrome/content/browser.js b/chrome/content/browser.js

>     let browsers = document.getElementById("browsers");
>     browsers.addEventListener("command", this._handleContentCommand, true);
>     browsers.addEventListener("DOMUpdatePageReport", gPopupBlockerObserver.onUpdatePageReport, false);
>+    browsers.addEventListener("focus", function() {
>+        selectedBrowser().messageManager.messageManager.sendAsyncMessage("Browser:Focus",{});
>+      }, true);
>+    browsers.addEventListener("blur", function() {
>+        selectedBrowser().messageManager.messageManager.sendAsyncMessage("Browser:Blur",{});
>+      }, true);

Browser.selectedBrowser.messageManager.sendAsyncMessage

should be the right way to get the message sent. Also, I wonder if the :Blur and :Focus messages should be moved to the browser.xml code?

Comment 3

[Jeremias Bosch (:jbos)]

Mhm, yes. I thought i tested it the upper way, but you are right... Might that i just changed it on device and didn't update my version working :)

I dont have a big feeling for one place or the other, so what ever works best - i would say :)

Comment 4

[Jeremias Bosch (:jbos)]

Attachment: Version 2

Fixed the comment. I thought about moving it to the xml, but i think its more obvious here. So i kept it in this place.

Comment 5

[Jeremias Bosch (:jbos)]

Its not that big patch, can we get it reviewed (+) :) soon?

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 476759 [details] [diff] [review]
Version 2

>diff --git a/chrome/content/browser.js b/chrome/content/browser.js

>     let browsers = document.getElementById("browsers");
>     browsers.addEventListener("command", this._handleContentCommand, true);
>     browsers.addEventListener("DOMUpdatePageReport", gPopupBlockerObserver.onUpdatePageReport, false);
>+    browsers.addEventListener("focus", function() {
>+        Browser.selectedBrowser.messageManager.messageManager.sendAsyncMessage("Browser:Focus",{});
>+      }, true);
>+    browsers.addEventListener("blur", function() {
>+        Browser.selectedBrowser.messageManager.messageManager.sendAsyncMessage("Browser:Blur",{});
>+      }, true);

I want to move these handlers, and _handleContentCommand, into a Browser.handleEvent method. It seems like clutter here. Someone should do that on checkin.

checkin after beta 1

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

I am worried about his patch. It is basically the same as bug 599053, which had problems too.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

I don't want to take this for b2. It could cause the same performance problems we saw in the original patch in bug 599053. (https://bugzilla.mozilla.org/attachment.cgi?id=478406&action=edit)

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 476759 [details] [diff] [review]
Version 2

This has already landed

Comment 10

[Stuart Parmenter]

Is there anything left to do here?  Do we need this?

Comment 11

[Jeremias Bosch (:jbos)]

Yes we need this for meego handset, it was removed somewhen last year. will add updated patch today and add it to meego meta bug.

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 476759 [details] [diff] [review]
Version 2

bitrotted

Comment 13

[Jeremias Bosch (:jbos)]

Attachment: updated

Updated Version

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

Not blocking N900 or Android releases

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

Not sure we need this anymore