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

VERIFIED FIXED

Status

MailNews Core
Feed Reader
--
minor
VERIFIED FIXED
13 years ago
7 years ago

People

(Reporter: Ger Teunis, Assigned: Magnus Melin)

Tracking

({fixed1.8.1})

1.8 Branch
fixed1.8.1
Bug Flags:
blocking1.8.0.1 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

13 years ago
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

13 years ago
Reproduced with TB 1.0+0603.
Severity: normal → minor
Status: UNCONFIRMED → RESOLVED
Last Resolved: 13 years ago
OS: Windows XP → Windows 2000
Resolution: --- → INVALID
Summary: RSS Subscriptions dialog doesn't have a paste context menu → Feed Properites (add subscription) dialog doesn't have a paste context menu
Version: unspecified → Trunk

Updated

13 years ago
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---

Comment 2

13 years ago
Sorry, I meant to confirm this, not to mark Invalid.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

13 years ago
Summary: Feed Properites (add subscription) dialog doesn't have a paste context menu → Feed Properties (add subscription) dialog doesn't have a paste context menu

Comment 3

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

Comment 4

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

Comment 5

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

Comment 6

12 years ago
(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

12 years ago
Reproduced.

Windows XP SP1
version 1.5 Beta 2 (20051011)

Comment 8

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

Comment 9

12 years ago
Reproduced.

SuSE Linux 9.3 
version 1.5 Beta 1 (20050908)

Updated

12 years ago
OS: Windows 2000 → All
Hardware: PC → All

Comment 10

12 years ago
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

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

Updated

12 years ago
Keywords: helpwanted

Comment 12

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

Comment 13

12 years ago
Considering this is the only way to add a feed, I think this is a pretty serious issue.
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?

Comment 14

12 years ago
Created attachment 207987 [details]
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

12 years ago
not a stopper for the security update.
Flags: blocking1.8.0.1? → blocking1.8.0.1-
(Assignee)

Comment 16

12 years ago
Created attachment 210066 [details] [diff] [review]
proposed fix

What Mike started...
Attachment #207987 - Attachment is obsolete: true
Attachment #210066 - Flags: review?(mscott)

Comment 17

12 years ago
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

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

Comment 19

12 years ago
(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".
Keywords: helpwanted

Comment 20

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

Comment 21

12 years ago
Created attachment 227821 [details]
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

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

Comment 23

12 years ago
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.
Attachment #210066 - Flags: review?(mscott)
Attachment #210066 - Flags: review+
Attachment #210066 - Flags: approval-thunderbird2+
(Assignee)

Comment 24

12 years ago
Created attachment 233352 [details] [diff] [review]
proposed fix, v2 (updated for checkin)

Updated the patch to address review comments.
Assignee: mscott → mkmelin+mozilla
Attachment #210066 - Attachment is obsolete: true
Attachment #227821 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Assignee)

Updated

12 years ago
QA Contact: mscott
Whiteboard: [checkin needed] [checkin needed (1.8 branch)]

Updated

12 years ago
Attachment #233352 - Flags: superreview+
Attachment #233352 - Flags: review+
Attachment #233352 - Flags: approval-thunderbird2+

Comment 25

12 years ago
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
Whiteboard: [checkin needed] [checkin needed (1.8 branch)] → [checkin needed (1.8 branch)]

Comment 26

12 years ago
This bug is very annoying. Could someone check it for new minor release?
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
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [checkin needed (1.8 branch)]
Target Milestone: --- → Thunderbird2.0
Version: Trunk → 2.0

Comment 28

11 years ago
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.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 29

11 years ago
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... 

Updated

9 years ago
Component: RSS → Feed Reader
Product: Thunderbird → MailNews Core
QA Contact: mscott → rss
Target Milestone: Thunderbird2.0 → ---

Updated

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