Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Add an ID to the rows element of editBookmarkPanelGrid in editBookmarkOverlay.xul

RESOLVED FIXED in Firefox 3.6a1

Status

()

Firefox
Bookmarks & History
--
enhancement
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: zydejazzman, Assigned: Margaret)

Tracking

Trunk
Firefox 3.6a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6

To assist extension developers who are trying to add overlays to the Places Library, please add an id to the columns and rows children of the editBookmarkPanelGrid grid.

Reproducible: Always

Steps to Reproduce:
1. View editBookmarkPanelGrid in DOM Inspector
2. No id is listed for the rows child of editBookmarkPanelGrid

Actual Results:  
No id is listed.

Expected Results:  
No id is listed.

Comment 1

9 years ago
This is a good idea I think.  Is there a reason we don't do this?
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

9 years ago
I should have checked before I posted, got this mixed up with something else.  It looks like every row child of the rows element in editBookmarkPanelGrid has an ID: editBMPanel_nameRow, editBMPanel_namePicker, editBMPanel_locationRow, etc.  The column elements do not have IDs, though.  Are you referring only to the columns?
(Reporter)

Comment 3

9 years ago
I was referring to the main rows container. I guess I misstated in my original request. The rows element is the one without the id; the children all have ids, as you have stated.

Comment 4

9 years ago
Oh, OK.  I know you wrote that you're trying to overlay the Places library, but what's the specific reason that having an ID on the rows element would help you?  If you need to insert a new row at a specific place, could you use insertbefore or insertafter relative to one of the existing child rows?  https://developer.mozilla.org/en/XUL_Tutorial/Overlays#Placing_Overlaid_Elements
OS: Windows XP → All
Hardware: x86 → All
Summary: Add an id to the rows child of editBookmarkPanelGrid → Add an ID to the rows element of editBookmarkPanelGrid in editBookmarkOverlay.xul
Whiteboard: [good first bug]

Comment 5

9 years ago
Uh, I just realized that you need the parent rows ID to use insertbefore/after, sorry.  So I revert to my original assessment that this is a good idea. :)

It's also really easy to fix.  Would you be up for writing a patch?  https://developer.mozilla.org/En/Creating_a_patch  If you need help, drop by #developers on IRC, or #places if you need Places-specific help.
(Assignee)

Comment 6

8 years ago
Created attachment 379780 [details] [diff] [review]
added ID to rows element

Updated

8 years ago
Attachment #379780 - Flags: review?(adw)

Comment 7

8 years ago
Comment on attachment 379780 [details] [diff] [review]
added ID to rows element

>diff --git a/browser/components/places/content/editBookmarkOverlay.xul b/browser/components/places/content/editBookmarkOverlay.xul
>--- a/browser/components/places/content/editBookmarkOverlay.xul
>+++ b/browser/components/places/content/editBookmarkOverlay.xul
>@@ -53,17 +53,17 @@
>       <label id="editBMPanel_itemsCountText"/>
>     </hbox>
> 
>     <grid id="editBookmarkPanelGrid" flex="1">
>       <columns>
>         <column/>
>         <column flex="1"/>
>       </columns>

Thanks Margaret!  While you're touching this code it would be useful for the columns element and the two column elements to have IDs too, IMHO.  Could you add those and request Marco's review on the new patch?

r=adw
Attachment #379780 - Flags: review?(adw) → review+
(Assignee)

Comment 8

8 years ago
Created attachment 379923 [details] [diff] [review]
added IDs to columns and column elements
Attachment #379780 - Attachment is obsolete: true
Attachment #379923 - Flags: review?(mak77)

Updated

8 years ago
Assignee: nobody → mleibovic
Status: NEW → ASSIGNED
Comment on attachment 379923 [details] [diff] [review]
added IDs to columns and column elements

I think you just want the second part of your patch here (it looks like the first version has the second version appended to it).

r=sdwilsh with that fix.
Attachment #379923 - Flags: review?(mak77) → review+
(Assignee)

Comment 10

8 years ago
Created attachment 379988 [details] [diff] [review]
patch
Attachment #379988 - Flags: review?(sdwilsh)
Comment on attachment 379988 [details] [diff] [review]
patch

bingo!

r=sdwilsh
Attachment #379988 - Flags: review?(sdwilsh) → review+

Updated

8 years ago
Keywords: checkin-needed
Target Milestone: --- → Firefox 3.6a1
Version: unspecified → Trunk
http://hg.mozilla.org/mozilla-central/rev/ee0809f3527d
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [good first bug]
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.