Closed Bug 512006 Opened 10 years ago Closed 10 years ago

factorize the way we handle ESCAPE to close dialogs

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: vingtetun, Assigned: vingtetun)

Details

Attachments

(1 file, 1 obsolete file)

The patch in bug 455866 add a new dialog, looking at the code, i have realized that all dialogs try to close on DOM_VK_ESCAPE and add an event listener for that.

This patch factorize that by assuming that each dialog which have registered through pushDialog implement a close method.
Comment on attachment 395978 [details] [diff] [review]
Patch v0.1

>+            if (aEvent.target == this._edit) {
>+              this._edit.reset();
>+              this._editToolbar(false);
>+            }

Nice cleanup. The only part I'd like to go further with is moving this part into the binding itself.

Why?, try this: open the awesomebar and press the "^" button. Notice we do _not_ reset the previous title caption back into the URLBar. It's really a separate bug, but if we can fix it here, all the better.
Comment on attachment 396193 [details] [diff] [review]
Patch v0.2

I've also removed the URLBAR_EDIT && URLBAR_FORCE flags since they are a bit unuseful now.
Attachment #395978 - Attachment is obsolete: true
Comment on attachment 396193 [details] [diff] [review]
Patch v0.2

>diff -r a1fd10fa40a7 chrome/content/bindings.xml

>       <method name="closePopup">

>           this.input.controller.stopSearch();
>+          this.input.reset();

I had to move the .reset into "close", since resetting in closePopup was messing up clicks in the awesomebar. The edit box was being reset and we lost the URL of the desired autocomplete item.

The close method is only called if we want to close the awesomebar without selecting an item.
Attachment #396193 - Flags: review?(mark.finkle) → review+
pushed: https://hg.mozilla.org/mobile-browser/rev/0dae7553447c
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #5)
> (From update of attachment 396193 [details] [diff] [review])
> >diff -r a1fd10fa40a7 chrome/content/bindings.xml
> 
> >       <method name="closePopup">
> 
> >           this.input.controller.stopSearch();
> >+          this.input.reset();
> 
> I had to move the .reset into "close", since resetting in closePopup was
> messing up clicks in the awesomebar. The edit box was being reset and we lost
> the URL of the desired autocomplete item.
> 
> The close method is only called if we want to close the awesomebar without
> selecting an item.


my bad. sorry for that.
bugspam
Assignee: nobody → 21
You need to log in before you can comment on or make changes to this bug.