Closed
Bug 175175
Opened 20 years ago
Closed 10 years ago
Add ID to Bookmark Menupopups
Categories
(SeaMonkey :: Bookmarks & History, defect)
Tracking
(seamonkey2.15 fixed)
RESOLVED
FIXED
seamonkey2.15
Tracking | Status | |
---|---|---|
seamonkey2.15 | --- | fixed |
People
(Reporter: andyed, Assigned: ewong)
References
Details
Attachments
(1 file, 5 obsolete files)
11.15 KB,
patch
|
ewong
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2b) Gecko/20021016 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2b) Gecko/20021016 Filing for news://news.netscape.com:119/aona02$35c1@ripley.netscape.com Needed to enable overlays... not sure about the debug menu, but the Bookmark menu is critical for add-on developers. Component reassignment welcome, bug 167391 addresses the GO menu and is assigned to no component. Reproducible: Always Steps to Reproduce: 1. View code 2. Settle for alternative overlay or hack *.xul directly
Reporter | ||
Comment 1•20 years ago
|
||
Focusing bug on bookmark menu per request in bug 161282
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Add ID's to Bookmark and Debug Menupopup 's → Add ID to Bookmark Menupopup 's
Since this is about bookmarks menupopups I am sendining this to bookmarks. I appologize if this is wrong.
Assignee: asa → ben
Component: Browser-General → Bookmarks
QA Contact: asa → claudius
Comment 3•18 years ago
|
||
This will patch the main menu in Mozilla and allow the BookmarksMenu to be extended with a dynamic overlay.
Comment 4•18 years ago
|
||
Comment on attachment 148322 [details] [diff] [review] This patch adds id attributes to the bookmark's menupopup and menuitems Could you please review this patch as a fix to allow extensions to use dynamic overlays to extend the BookmarksMenu from Mozilla's main menu? Thanks, Boyd
Attachment #148322 -
Flags: review?(bugs)
Comment 5•18 years ago
|
||
Ben doesn't work on Seamonkey (mozilla suite) stuff any more... might be best to request review from someone else.
Assignee: bugs → p_ch
QA Contact: claudius → seamonkey.bookmarks
Comment 6•18 years ago
|
||
Comment on attachment 148322 [details] [diff] [review] This patch adds id attributes to the bookmark's menupopup and menuitems ben is highly unlikely to review this. requesting review from default assignee.
Attachment #148322 -
Flags: review?(bugs) → review?(p_ch)
Updated•18 years ago
|
Product: Browser → Seamonkey
Comment 7•17 years ago
|
||
default assignee Pierre is no longer active with bookmarks boris, can you review - pc_h is no longer doing bookmark reviews
Assignee: p_ch → bzbarsky
Comment 8•17 years ago
|
||
Comment on attachment 148322 [details] [diff] [review] This patch adds id attributes to the bookmark's menupopup and menuitems Pierre not reviewing
Attachment #148322 -
Flags: review?(p_ch) → review?
Comment 9•17 years ago
|
||
Comment on attachment 148322 [details] [diff] [review] This patch adds id attributes to the bookmark's menupopup and menuitems Your menuitem ids aren't consistent, either match the groupmark menuitem or change the groupmark menuitem to match your ids.
![]() |
||
Comment 10•17 years ago
|
||
Excuse me? I have no idea why you decided to assign this bug to me. Please do NOT assign bugs to me unless they're regressions caused by a patch I checked in or unless I explicitly requested that they be assigned to me. If you want me to review, request review via the review flags. That said, I know nothing about bookmarks or the bookmarks UI, and I'm not a module owner or peer for any of it, so I can't review that patch anyway. See http://www.mozilla.org/owners.html for a list of people you can ask for review.
Assignee: bzbarsky → nobody
Updated•17 years ago
|
Summary: Add ID to Bookmark Menupopup 's → Add ID to Bookmark Menupopups
Comment 11•17 years ago
|
||
Tim, are you available to update the patch per comment 9?
Updated•17 years ago
|
Attachment #148322 -
Flags: review?
Comment 12•17 years ago
|
||
(In reply to comment #11) > Tim, are you available to update the patch per comment 9? I will make some time next week to update the patch.
Comment 13•17 years ago
|
||
bump
Attachment #148322 -
Flags: review?(timeless)
Comment 14•16 years ago
|
||
Comment on attachment 148322 [details] [diff] [review] This patch adds id attributes to the bookmark's menupopup and menuitems per Comment 9
Attachment #148322 -
Flags: review?(timeless) → review-
![]() |
Assignee | |
Comment 15•10 years ago
|
||
Assignee: nobody → ewong
Attachment #148322 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #653689 -
Flags: review?(mnyromyr)
Comment 16•10 years ago
|
||
Comment on attachment 653689 [details] [diff] [review] Add IDs to Bookmark menupopups. (v1) >- <menuitem id="menuitem_showhide_tabbar" >+ <menuitem id="menuitem_showHide_tabBar" This is wrong — there's no point here in changing an existing id. >+ <menuitem id="menuitem_componentBarCmd" >+ label="&componentbarCmd.label;" I'd prefer the new id's case to match the label's, i.e. menuitem_componentbarCmd here. >- <menu id="menu_UseStyleSheet" >+ <menu id="menu_useStylesheet" No. :( Normal diff unfortunately doesn't mark the exact change, only lines — but e.g. kdiff3 does and may help to catch such changes.
Attachment #653689 -
Flags: review?(mnyromyr) → review-
![]() |
Assignee | |
Comment 17•10 years ago
|
||
Attachment #653689 -
Attachment is obsolete: true
Attachment #657523 -
Flags: review?(mnyromyr)
![]() |
||
Comment 18•10 years ago
|
||
> - <menuitem label="&stopCmd.label;" accesskey="&stopCmd.accesskey;" id="menuitem-stop" disabled="true" oncommand="BrowserStop();" key="key_stop"/>
> + <menuitem id="menuitem_stopCmd"
> + label="&stopCmd.label;"
> + accesskey="&stopCmd.accesskey;"
> + id="menuitem-stop"
> + disabled="true"
> + oncommand="BrowserStop();"
> + key="key_stop"/>
This item already has an ID of "menuitem-stop"!
![]() |
Assignee | |
Comment 19•10 years ago
|
||
Attachment #657523 -
Attachment is obsolete: true
Attachment #657523 -
Flags: review?(mnyromyr)
Attachment #657625 -
Flags: review?(mnyromyr)
Comment 20•10 years ago
|
||
Comment on attachment 657625 [details] [diff] [review] Add IDs to Bookmark menu popups. (v3) >- <menuitem id="menuitem_showhide_tabbar" label="&tabbarCmd.label;" accesskey="&tabbarCmd.accesskey;" class="menuitem-iconic" type="checkbox" oncommand="showHideTabbar();" checked="true"/> >+ <menuitem id="menuitem_showhide_tabBar" Sorry, but you are still altering an existing id. :( >- <menuitem label="&stopCmd.label;" accesskey="&stopCmd.accesskey;" id="menuitem-stop" disabled="true" oncommand="BrowserStop();" key="key_stop"/> >+ <menuitem label="&stopCmd.label;" >+ accesskey="&stopCmd.accesskey;" >+ id="menuitem-stop" >+ disabled="true" >+ oncommand="BrowserStop();" >+ key="key_stop"/> Please make id the first attribute like everywhere else. (Nice spotting, Phil!) >+ <menuitem id="menuitem_pageSourceCmd" >+ accesskey="&pageSourceCmd.accesskey;" >+ label="&pageSourceCmd.label;" >+ key="key_viewSource" >+ command="View:PageSource"/> >+ <menuitem id="menuitem_pageInfoCmd" >+ accesskey="&pageInfoCmd.accesskey;" >+ label="&pageInfoCmd.label;" >+ key="key_viewInfo" >+ command="View:PageInfo"/> Please move label to second position. >+ <menu id="menu_iconic_feedsMenu" >+ class="menu-iconic feedsMenu" >+ command="feedsMenu" >+ label="&feedsMenu.label;" >+ accesskey="&feedsMenu.accesskey;"> Please move label/accesskey to second/third position. r- for the one unwanted id change.
Attachment #657625 -
Flags: review?(mnyromyr) → review-
![]() |
Assignee | |
Comment 21•10 years ago
|
||
Attachment #657625 -
Attachment is obsolete: true
Attachment #658336 -
Flags: review?(mnyromyr)
Comment 22•10 years ago
|
||
Comment on attachment 658336 [details] [diff] [review] Add IDs to Bookmark Menupopups (v4) > <menuitem id="menuitem_showhide_tabbar" label="&tabbarCmd.label;" accesskey="&tabbarCmd.accesskey;" class="menuitem-iconic" type="checkbox" oncommand="showHideTabbar();" checked="true"/> Might as well rewrap this while you're here (if you feel like). >+ <menuitem id="menuitem_taskbarCmd" >+ label="&taskbarCmd.label;" >+ accesskey="&taskbarCmd.accesskey;" >+ class="menuitem-iconic" >+ type="checkbox" >+ observes="cmd_viewtaskbar" /> Please remove the space before /> on checkin. r=me.
Attachment #658336 -
Flags: review?(mnyromyr) → review+
![]() |
Assignee | |
Comment 23•10 years ago
|
||
Attachment #658336 -
Attachment is obsolete: true
Attachment #659081 -
Flags: review+
![]() |
Assignee | |
Comment 24•10 years ago
|
||
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/f6cce22f417f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 25•10 years ago
|
||
Settings flags to make tracking of fixed bugs easier.
status-seamonkey2.15:
--- → fixed
Target Milestone: --- → seamonkey2.15
You need to log in
before you can comment on or make changes to this bug.
Description
•