296542 - Feed Properties (add subscription) dialog doesn't have a paste context menu

Status: VERIFIED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Ger Teunis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050603 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050603 Firefox/1.0+

The Feed URL only has an "Copy link location" context menu.
It would really be nice to provide a Paste link location as well.
Most people use the copy/paste from the contextmenu.

Reproducible: Always

Steps to Reproduce:
1. Copy a RSS feed location (to the clipboard)
2. Open the RSS Subscriptions dialog
3. Try pasting it without using the keyboard (thus with the rightclick menu)

Actual Results:  
Only one item appears: "Copy link location"

Expected Results:  
At least the options "Paste link location" and "Copy link location" should
appear (even a Cut/Clear option?)

I think a lot of users don't know the CTRL-C and V shortcuts and would like to
use the context menu of the location URL edit box for pasting links.

Comment 1

[Mike Cowperthwaite]

Reproduced with TB 1.0+0603.

Comment 2

[Mike Cowperthwaite]

Sorry, I meant to confirm this, not to mark Invalid.

Comment 3

[hansen]

*** Bug 302631 has been marked as a duplicate of this bug. ***

Comment 4

[Mike Cowperthwaite]

*** Bug 308440 has been marked as a duplicate of this bug. ***

Comment 5

[Grega Fajdiga]

I cannot seem to paste the link via CTRL-V either.

Comment 6

[Grega Fajdiga]

(In reply to comment #5)
> I cannot seem to paste the link via CTRL-V either.

Sorry, false alarm. Ctrl-V miraculously started working :-)

Comment 7

[Hiro]

Reproduced.

Windows XP SP1
version 1.5 Beta 2 (20051011)

Comment 8

[Mike Cowperthwaite]

*** Bug 312987 has been marked as a duplicate of this bug. ***

Comment 9

[Mordechai Peller]

Reproduced.

SuSE Linux 9.3 
version 1.5 Beta 1 (20050908)

Comment 10

[Mike Cowperthwaite]

This bug has been confirmed.  We do not need anyone to report that the bug 
still exists.  If you want to be helpful, write a patch.

Comment 11

[Mike Cowperthwaite]

*** Bug 320360 has been marked as a duplicate of this bug. ***

Comment 12

[Adam Guthrie]

*** Bug 322714 has been marked as a duplicate of this bug. ***

Comment 13

[Adam Guthrie]

Considering this is the only way to add a feed, I think this is a pretty serious issue.

Comment 14

[Mike Cowperthwaite]

Attachment: First draft, not working (diff against 1.6a1-0105)

Here's a basic approach to fixing this.  The problem is that the same dialog is used for both adding a feed and editing it; when you edit it, the URL field is read-only, so the Copy context menu is the only that makes sense.  But the dialog is written to simply always assign that menu to the field.

In this patch, I've removed the menu assignment from the template and tried to apply it for the Edit Feed case.  It's close to correct: the code change does change the context menu, but it changes it to "no menu" at all, rather than retrieving the menu I expected.  I'm sure there's some simple XUL incantation that I'm simply not making.

Comment 15

[Scott MacGregor]

not a stopper for the security update.

Comment 16

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

What Mike started...

Comment 17

[Mike Cowperthwaite]

Did you test that?  Does it work as expected?

I don't think it's necessary to have a separate "pasteURLpopup" -- if the 
field is editable, it should just have the default edit-field popup, and with the patch I wrote, that's exactly what I got.  The problem I was having was getting the copy-only URL to appear when the field was disabled.

Comment 18

[Oliver Saier]

Yes the patch is working fine for me (applied to the 1/29 branch nightly)

Comment 19

[Magnus Melin [:mkmelin]]

(In reply to comment #17)
> Did you test that?  Does it work as expected?

Of course:)

> I don't think it's necessary to have a separate "pasteURLpopup" -- if the 
> field is editable, it should just have the default edit-field popup, and with
> the patch I wrote, that's exactly what I got.  The problem I was having was
> getting the copy-only URL to appear when the field was disabled.

You had an extra document.getElementBy... It *could* also work without the custom paste popup, but the default one you get there is very crowded - everything from "spell check this field" to select all... So I added a custom one. I can make another patch with the default context menu if that's preferred. 

We use a custom "copy" popup menu, but that is of course somewhat different since it basically does "select all + copy".

Comment 20

[Mike Cowperthwaite]

*** Bug 335503 has been marked as a duplicate of this bug. ***

Comment 21

[Oliver Saier]

Attachment: Extension that implements the patch

For some reason I had trouble with oncommand="cmd_paste" so I replaced it with oncommand="goDoCommand('cmd_paste')". Also, I used "Paste Link Location" for consistency with "Copy Link Location", but shouldn't it be "Paste Feed Location"?

Comment 22

[Oliver Saier]

Discard my comments Copy Link Location is how it is in Firefox, and goDoCommand is how it is in the patch anyway :)

Comment 23

[Scott MacGregor]

Comment on attachment 210066 [details] [diff] [review]
proposed fix

can you add a newline at the end of feed-properties.dtd?

I think the menu item should be just Paste.

Comment 24

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2 (updated for checkin)

Updated the patch to address review comments.

Comment 25

[Peter Kasting]

Checked in on trunk.  Not checking in on 1.8 branch due to closure for FxB2.

/mozilla/mail/locales/en-US/chrome/messenger-newsblog/feed-properties.dtd 1.5
/mozilla/mail/extensions/newsblog/content/feed-properties.js 1.12
/mozilla/mail/extensions/newsblog/content/feed-properties.xul 1.13

Comment 26

[Sorin Sbarnea]

This bug is very annoying. Could someone check it for new minor release?

Comment 27

[:Gavin Sharp [email: [e-mail]]]

mozilla/mail/extensions/newsblog/content/feed-properties.js 1.11.4.1
mozilla/mail/locales/en-US/chrome/messenger-newsblog/feed-properties.dtd 1.4.4.1
mozilla/mail/extensions/newsblog/content/feed-properties.xul 1.12.4.1

Comment 28

[Mike Cowperthwaite]

V with TB 2a1-0828.

I'm still a bit apprehensive about this menu being custom -- but I don't generally find the context menu on an edit field to be much use anyway.  For the six default commands, I almost always use keyboard shortcuts, with Paste being the most frequent exception.

Magnus, did you really find that the default menu included Spell Check (per comment 19)?  I would expect the default to be the same as that seen on the Subject field in a compose window, or the Quick Search, or any of the edit fields in the Options dialog.

Comment 29

[Magnus Melin [:mkmelin]]

Mike: yeah, I'm pretty sure I saw it at the time but when testing now I don't anymore. Must have been some odd situation at the trunk at the time... And now when I saw what it looked like, I too would prefer the default popup. Maybe someone should file a bug...