Closed
Bug 512006
Opened 14 years ago
Closed 14 years ago
factorize the way we handle ESCAPE to close dialogs
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: vingtetun, Assigned: vingtetun)
Details
Attachments
(1 file, 1 obsolete file)
7.85 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
Comment 2•14 years ago
|
||
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.
Assignee | ||
Comment 3•14 years ago
|
||
Assignee | ||
Comment 4•14 years ago
|
||
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.
Assignee | ||
Updated•14 years ago
|
Attachment #396193 -
Flags: review?(mark.finkle)
Updated•14 years ago
|
Attachment #395978 -
Attachment is obsolete: true
Comment 5•14 years ago
|
||
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+
Comment 6•14 years ago
|
||
pushed: https://hg.mozilla.org/mobile-browser/rev/0dae7553447c
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•14 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•