Site Panel does not close when clicking on Find in Page

VERIFIED FIXED

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: aakashd, Assigned: mbrubeck)

Tracking

Bug Flags:
in-testsuite +

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Build Id:
Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b6pre) Gecko/20100930 Namoroka/4.0b7pre Fennec/4.0b1pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b6pre) Gecko/20100930 Namoroka/4.0b7pre Fennec/4.0b1pre

Steps to Reproduce:
1. Click on the favicon left of the url bar
2. Click on "Find in Page"

Actual Results:
The site panel is not closed

Expected Results:
The site panel is closed.
(Reporter)

Updated

8 years ago
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0+
(Assignee)

Updated

8 years ago
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Blocks: 596619
(Assignee)

Comment 1

8 years ago
Created attachment 479872 [details] [diff] [review]
patch

This is a regression from bug 596619.  Here's a simple fix and a new regression test.
Attachment #479872 - Flags: review?(mark.finkle)
Comment on attachment 479872 [details] [diff] [review]
patch

>diff -r bedc4384560b chrome/content/browser-ui.js

>   show: function findHelperShow() {
>+    BrowserUI._hidePopup();
>     this._container.show(this);
>     this.search("");

I don't like managing the popups manually like this. I wonder if we should be just closing the Site Menu after any Page Action is fired? Do we ever want the Site Menu to stay open?

>diff -r bedc4384560b chrome/tests/browser_find.js

>+function test() {
>+  let menu = document.getElementById('identity-container');
>+  let item = document.getElementById('pageaction-findinpage');
>+  let navigator = document.getElementById('content-navigator');

Use "
(Assignee)

Comment 3

8 years ago
(In reply to comment #2)
> I don't like managing the popups manually like this. I wonder if we should be
> just closing the Site Menu after any Page Action is fired? Do we ever want the
> Site Menu to stay open?

I'd be in favor of closing the site menu automatically on click.

The one case where we might want the site menu to stay open is for the "Add Search Engine" action, which modifies the menu in place.  If so, its click handler could use event.stopPropagation() to prevent its click from bubbling up to the menu.

I'll try this out...
(Assignee)

Comment 4

8 years ago
Created attachment 479889 [details] [diff] [review]
patch v2

(In reply to comment #2)
> I don't like managing the popups manually like this. I wonder if we should be
> just closing the Site Menu after any Page Action is fired? Do we ever want the
> Site Menu to stay open?

Updated the patch to close the site menu when any page action is clicked, and updated three page actions (Add Search Engine, Forget Password, Clear Permissions) to keep their current behavior of *not* closing the menu.

> Use "

Fixed.
Attachment #479872 - Attachment is obsolete: true
Attachment #479889 - Flags: review?(mark.finkle)
Attachment #479872 - Flags: review?(mark.finkle)
Comment on attachment 479889 [details] [diff] [review]
patch v2

Thanks Matt!
Attachment #479889 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 6

8 years ago
http://hg.mozilla.org/mobile-browser/rev/15f6e169870b
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Flags: in-testsuite+
Verified:
Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b7pre)Gecko/20101001 Firefox/4.0b7pre Fennec/4.0b2pre

Mozilla/5.0 (Android; Linux armv71; rv2.0b7pre) Gecko/20101001 Firefox/4.0b7pre Fennec/4.0b2pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.