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

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, enhancement)

Description

[zydejazzman]

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

[Drew Willcoxon :adw]

This is a good idea I think.  Is there a reason we don't do this?

Comment 2

[Drew Willcoxon :adw]

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?

Comment 3

[zydejazzman]

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

[Drew Willcoxon :adw]

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

Comment 5

[Drew Willcoxon :adw]

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.

Comment 6

[:Margaret Leibovic]

Attachment: added ID to rows element

Comment 7

[Drew Willcoxon :adw]

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

Comment 8

[:Margaret Leibovic]

Attachment: added IDs to columns and column elements

Comment 9

[Shawn Wilsher :sdwilsh]

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.

Comment 10

[:Margaret Leibovic]

Attachment: patch

Comment 11

[Shawn Wilsher :sdwilsh]

Comment on attachment 379988 [details] [diff] [review]
patch

bingo!

r=sdwilsh

Comment 12

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/ee0809f3527d

Comment 13

[Gervase Markham [:gerv]]

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