Last Comment Bug 654601 - cannot use copy and paste keyboard shortcuts in panorama
: cannot use copy and paste keyboard shortcuts in panorama
Status: VERIFIED FIXED
: regression
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: x86 All
: -- normal
: Firefox 7
Assigned To: Raymond Lee [:raymondlee]
:
:
Mentors:
Depends on:
Blocks: 587276 660175
  Show dependency treegraph
 
Reported: 2011-05-03 16:12 PDT by tinagma
Modified: 2016-04-12 14:00 PDT (History)
11 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (2.49 KB, patch)
2011-05-03 22:44 PDT, Raymond Lee [:raymondlee]
ian: review+
ttaubert: feedback+
Details | Diff | Splinter Review
Patch for checkin (2.75 KB, patch)
2011-05-18 23:18 PDT, Raymond Lee [:raymondlee]
asa: approval‑mozilla‑aurora+
jst: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description tinagma 2011-05-03 16:12:24 PDT
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a1) Gecko/20110429 Firefox/6.0a1
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a1) Gecko/20110429 Firefox/6.0a1

When trying to cut, copy, or paste to the titles in panorama using the shortcut keys, nothing happens. I can do it just fine using menu commands, but this is different from the entire rest of the browser.

Reproducible: Always

Steps to Reproduce:
1.open panorama
2.name a tab group
3.select the tab title
4.press cmd-x

Actual Results:  
nothing happens

Expected Results:  
the text should be removed and placed on the clipboard
Comment 1 tinagma 2011-05-03 16:40:07 PDT
note: nothing happens when cmd-c or cmd-v is tried, either.
Comment 2 Alice0775 White 2011-05-03 16:52:29 PDT
Regression window(m-c hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/76fbb32b78af
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.2a1pre) Gecko/20110331 Firefox/4.2a1pre ID:20110331104953
Fails:
http://hg.mozilla.org/mozilla-central/rev/e38b294f02c5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.2a1pre) Gecko/20110331 Firefox/4.2a1pre ID:20110331125525
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=76fbb32b78af&tochange=e38b294f02c5
Suspected:
565c588e3e51	Raymond Lee — Bug 587276 - KeyEvent in TabCandy window affect browser [r=ian]
Comment 3 Raymond Lee [:raymondlee] 2011-05-03 22:44:14 PDT
Created attachment 529930 [details] [diff] [review]
v1

Re-enable cut, copy, paste, undo, redo and selectAll
Comment 4 Tim Taubert [:ttaubert] 2011-05-04 04:07:52 PDT
Comment on attachment 529930 [details] [diff] [review]
v1

Review of attachment 529930 [details] [diff] [review]:

Looks good. The only thing I don't like (about the solution in general) is that we can't allow shortcuts that have no associated command like (Ctrl+BackSpace, Ctrl+Left, Ctrl+Right, etc.). But seems there is no way to integrate them with our current white list implementation.
Comment 5 Johnathan Nightingale [:johnath] 2011-05-04 11:49:14 PDT
This goes on the tracking list for FF5.

Can I get a risk assessment: should we be fixing this on aurora, or backing out the change that caused it?
Comment 6 Tim Taubert [:ttaubert] 2011-05-04 11:59:17 PDT
(In reply to comment #5)
> Can I get a risk assessment: should we be fixing this on aurora, or backing out
> the change that caused it?

The risk of this patch is really low as it only adds some new commands to the white list. It re-enables cut, copy, paste, undo, redo and selectAll but not [Ctrl+BackSpace/Left/Right]. So if the latter ones are really important we might have to back out.
Comment 7 Ian Gilman [:iangilman] 2011-05-05 13:55:18 PDT
Comment on attachment 529930 [details] [diff] [review]
v1

Review of attachment 529930 [details] [diff] [review]:

I agree with Tim; it would be good to support those additional shortcuts. The patch as is is a good fix, though; maybe file a follow up?
Comment 8 Raymond Lee [:raymondlee] 2011-05-05 14:19:57 PDT
(In reply to comment #7)
> Comment on attachment 529930 [details] [diff] [review] [review]
> v1
> 
> Review of attachment 529930 [details] [diff] [review] [review]:
> 
> I agree with Tim; it would be good to support those additional shortcuts. The
> patch as is is a good fix, though; maybe file a follow up?

I am working on a patch for bug 621795 which should also fix this bug.  I suggest to put this patch on hold for now.
Comment 9 Peter van der Woude [:Peter6] 2011-05-17 13:53:37 PDT
is this the bug that would cover not being able to paste anything into the find-field of panorama or would that require a new bug ?
Comment 10 Tim Taubert [:ttaubert] 2011-05-17 13:56:31 PDT
(In reply to comment #9)
> is this the bug that would cover not being able to paste anything into the
> find-field of panorama or would that require a new bug ?

I think we can handle this here as the patch will very likely fix this, too.
Comment 11 Asa Dotzler [:asa] 2011-05-18 22:45:07 PDT
What we have here is a reviewed fix in hand that covers a good subset of the problem case. Over in bug 621795 we don't have a reviewed patch and what's there seems considerably more involved. 

We should either take this patch into Beta (in which case, can one of you please nominate it with approval-mozilla-beta?) or we should re-visit backing out what ever change caused this regression. (What change was that?)
Comment 12 Raymond Lee [:raymondlee] 2011-05-18 23:18:40 PDT
Created attachment 533544 [details] [diff] [review]
Patch for checkin

Submitted to try and waiting for the result.
http://tbpl.mozilla.org/?tree=Try&rev=15939812e38d
Comment 13 Raymond Lee [:raymondlee] 2011-05-19 08:24:22 PDT
(In reply to comment #12)
> Created attachment 533544 [details] [diff] [review] [review]
> Patch for checkin
> 
> Submitted to try and waiting for the result.
> http://tbpl.mozilla.org/?tree=Try&rev=15939812e38d

Passed Try!
Comment 14 Asa Dotzler [:asa] 2011-05-19 15:15:46 PDT
Comment on attachment 533544 [details] [diff] [review]
Patch for checkin

Please land this change on both Aurora and Beta. (In the future, getting changes in during Aurora will save you this extra step.)
Comment 15 Daniel Holbert [:dholbert] 2011-05-22 16:23:18 PDT
RE "checkin-needed" -- does this want to land on m-c, too, or just aurora/beta?
Comment 16 Raymond Lee [:raymondlee] 2011-05-22 20:27:45 PDT
(In reply to comment #15)
> RE "checkin-needed" -- does this want to land on m-c, too, or just
> aurora/beta?

Please land on m-c as well.  Thanks!
Comment 17 Dão Gottwald [:dao] 2011-05-25 02:54:08 PDT
http://hg.mozilla.org/mozilla-central/rev/d0071580fb55
Comment 18 Tim Taubert [:ttaubert] 2011-05-25 03:51:17 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/3410d77f65aa
Comment 19 Tim Taubert [:ttaubert] 2011-05-25 04:37:39 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/5b19bad76854
Comment 20 Simona B [:simonab ] 2011-05-26 05:03:19 PDT
Mozilla/5.0 (Windows NT 5.1; rv:6.0a2) Gecko/20110525 Firefox/6.0a2

Checked it on WinXP, Win 7, Mac OSX, Ubuntu.
Using the steps to reproduce from Comment 0, I was able to cut, copy, paste, undo, selectAll but not able to redo (ctrl+y/cmd+y) any of the actions. 

Reopening issue.
Comment 21 Tim Taubert [:ttaubert] 2011-05-26 05:07:12 PDT
They're different on unix and mac/win. We're unfortunately supporting unix only atm.

> #ifdef XP_UNIX
>     <key id="key_redo" key="&undoCmd.key;" modifiers="accel,shift"/>
> #else
>     <key id="key_redo" key="&redoCmd.key;" modifiers="accel"/>
> #endif
Comment 22 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-05-26 10:05:41 PDT
Based on comment 21, I think this can be re-resolved and a new one open specific to the "redo" keybinding. Tim?
Comment 23 Raymond Lee [:raymondlee] 2011-05-26 10:50:35 PDT
Filed bug 660017
Comment 24 Peter van der Woude [:Peter6] 2011-05-26 11:12:45 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > is this the bug that would cover not being able to paste anything into the
> > find-field of panorama or would that require a new bug ?
> 
> I think we can handle this here as the patch will very likely fix this, too.

The context menu of the findbar is still missing, new bug blocking this one or Bug 653099 ?
Comment 25 Raymond Lee [:raymondlee] 2011-05-26 11:19:32 PDT
Context menu is a different topic.  It should block bug 653099 and bug 590251
Comment 26 Tim Taubert [:ttaubert] 2011-05-27 02:09:50 PDT
bugspam
Comment 27 George Carstoiu 2011-06-21 05:49:16 PDT
Verified on Ubuntu 11.04 x86, Mac OS X 10.6, Win 7 x86 and WinXP.

With the exception of CTRL+Y (redo) which has it's  own bug 660017, keyboard shortcuts work within Panorama. Setting status to Verified Fixed.

Note You need to log in before you can comment on or make changes to this bug.