Closed
Bug 955484
Opened 11 years ago
Closed 11 years ago
Escape key should close awesometab
Categories
(Instantbird Graveyard :: Other, defect)
Instantbird Graveyard
Other
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: aleth, Assigned: nhnt11)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2047 at 2013-07-12 15:02:00 UTC ***
For consistency with conv tabs
Assignee | ||
Comment 1•11 years ago
|
||
*** Original post on bio 2047 as attmnt 2585 at 2013-07-12 15:11:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354354 -
Flags: review?(aleth)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
Comment on attachment 8354354 [details] [diff] [review]
Escape key closes awesometab if filterbox is empty. Else, clear filterbox (default behavior)
*** Original change on bio 2047 attmnt 2585 at 2013-07-12 15:13:03 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354354 -
Attachment is patch: true
Attachment #8354354 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 3•11 years ago
|
||
*** Original post on bio 2047 as attmnt 2586 at 2013-07-12 15:14:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354355 -
Flags: review?(aleth)
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8354354 [details] [diff] [review]
Escape key closes awesometab if filterbox is empty. Else, clear filterbox (default behavior)
*** Original change on bio 2047 attmnt 2585 at 2013-07-12 15:14:57 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354354 -
Attachment is obsolete: true
Attachment #8354354 -
Flags: review?(aleth)
Reporter | ||
Comment 5•11 years ago
|
||
Comment on attachment 8354355 [details] [diff] [review]
Fix indentation
*** Original change on bio 2047 attmnt 2586 at 2013-07-12 15:25:33 UTC ***
I don't think <key>s correspond to keydown events, so you can save yourself the extra handler.
http://lxr.instantbird.org/instantbird/source/instantbird/content/instantbird.xul#83
Attachment #8354355 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 6•11 years ago
|
||
*** Original post on bio 2047 as attmnt 2587 at 2013-07-12 16:06:00 UTC ***
This enables "escape to close" for all tabs.
In awesometabs, when there is text entered in the filterbox, pressing escape clears it and the command is not invoked.
Attachment #8354356 -
Flags: review?(florian)
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 8354355 [details] [diff] [review]
Fix indentation
*** Original change on bio 2047 attmnt 2586 at 2013-07-12 16:06:59 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354355 -
Attachment is obsolete: true
Comment 8•11 years ago
|
||
*** Original post on bio 2047 at 2013-07-12 16:32:38 UTC ***
(In reply to comment #4)
> Created attachment 8354356 [details] [diff] [review] (bio-attmnt 2587) [details]
> Better solution
>
> This enables "escape to close" for all tabs.
> In awesometabs, when there is text entered in the filterbox, pressing escape
> clears it and the command is not invoked.
A.k.a. "this might lose data on any tab except for conversations which are put on hold"? ;)
I don't think we should do this in general.
Reporter | ||
Comment 9•11 years ago
|
||
Comment on attachment 8354356 [details] [diff] [review]
Better solution
*** Original change on bio 2047 attmnt 2587 at 2013-07-13 11:59:37 UTC ***
It's no good to change the command in this way and still call it cmd_putOnHold, as I mentioned on IRC.
But before you change this, it seems we need to discuss with everyone on IRC what the expected behaviour for ESC is, as there seem to be some differing opinions ;)
Attachment #8354356 -
Flags: review?(florian) → review-
Comment 10•11 years ago
|
||
*** Original post on bio 2047 at 2013-07-13 22:00:36 UTC ***
(In reply to comment #6)
> But before you change this, it seems we need to discuss with everyone on IRC
> what the expected behaviour for ESC is, as there seem to be some differing
> opinions ;)
This seems like it would take us a while to reach complete agreement. Is there any reason why a solution along the lines of attachment 8354355 [details] [diff] [review] (bio-attmnt 2586) (after addressing comment 3) wouldn't be acceptable?
Reporter | ||
Comment 11•11 years ago
|
||
*** Original post on bio 2047 at 2013-07-15 09:01:32 UTC ***
(In reply to comment #7)
> (In reply to comment #6)
>
> > But before you change this, it seems we need to discuss with everyone on IRC
> > what the expected behaviour for ESC is, as there seem to be some differing
> > opinions ;)
>
> This seems like it would take us a while to reach complete agreement. Is there
> any reason why a solution along the lines of attachment 8354355 [details] [diff] [review] (bio-attmnt 2586) [details] (after addressing
> comment 3) wouldn't be acceptable?
It would be fine to do something along the lines of that attachment, but in the existing awesometab keyboard handlers. It's a bit odd though to add ESC here so it "works everywhere", but not to code it so it actually works everywhere (e.g. it will not close about:* tabs). I guess we can live with that for now, as we don't seem to have decided what "works" means in general here ;)
Assignee | ||
Comment 12•11 years ago
|
||
*** Original post on bio 2047 as attmnt 2599 at 2013-07-15 19:36:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354368 -
Flags: review?(florian)
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 8354356 [details] [diff] [review]
Better solution
*** Original change on bio 2047 attmnt 2587 at 2013-07-15 19:36:43 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354356 -
Attachment is obsolete: true
Comment 14•11 years ago
|
||
Comment on attachment 8354368 [details] [diff] [review]
Don't handle escape for all tabs. Use existing keydown handler in newtab.xml.
*** Original change on bio 2047 attmnt 2599 at 2013-07-15 20:22:14 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354368 -
Flags: review?(florian) → review+
Comment 15•11 years ago
|
||
*** Original post on bio 2047 at 2013-07-15 21:27:12 UTC ***
http://hg.instantbird.org/instantbird/rev/d04931876b00
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5
You need to log in
before you can comment on or make changes to this bug.
Description
•