Closed Bug 282187 Opened 17 years ago Closed 15 years ago

sync xpfe textbox.xml with toolkit textbox.xml

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.8beta3

People

(Reporter: mconnor, Assigned: sgautherie)

References

Details

(Keywords: helpwanted, Whiteboard: [SergeG: helpwanted to port bug 302050])

Attachments

(5 obsolete files)

 
Blocks: 282177
URL:
Depends on: 280485
Quoted from <http://wiki.mozilla.org/wiki/User:Biesi>
{{
 "Redo" in ctxt menu of textboxes will be lost, probably not much of a loss
}}
(I wonder why Redo in not there in Toolkit ? I guess there is a reason...)
Attached patch 2005.03.09 X2T diff report (obsolete) — Splinter Review
Diff from Xpfe to Toolkit version.

These came from Toolkit versions:
{{
1.16	ben%bengoodger.com	2004-11-30 20:48		Aviary branch
landing
1.10	blakeross%telocity.com	2003-08-27 01:24		remove "redo"
item that we got from mozilla.
}}
(In reply to comment #1)
> Quoted from <http://wiki.mozilla.org/wiki/User:Biesi>
> {{
>  "Redo" in ctxt menu of textboxes will be lost, probably not much of a loss
> }}
> (I wonder why Redo in not there in Toolkit ? I guess there is a reason...)

Brian, Neil:
Could you comment/confirm on what should be done with "Redo":
remove it from Xpfe, or add it (back) to Toolkit !?

Neil:
Can |anonid="input-box-contextmenu"| be added safely to Xpfe, or is anything
else needed ?
Assignee: nobody → gautheri
Keywords: helpwanted
Clears the way to better see the 2 remaining sync to do.

Could you (super-)review/check in this patch ? Thanks.

Then, would you have time to comment on my comment 3 questions ?
Attachment #178036 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #178036 - Flags: review?(neil.parkwaycc.co.uk)
(In reply to comment #3)
>remove it from Xpfe
IIRC it was timeless's idea so unless he can convince the Sea Lords...

>Can |anonid="input-box-contextmenu"| be added safely to Xpfe
Sure, it was only added for the convenience of the FireFox search bar.
Attachment #178036 - Attachment is obsolete: true
Attachment #178081 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #178081 - Flags: review?(mconnor)
Attachment #178036 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #178036 - Flags: review?(neil.parkwaycc.co.uk)
(In reply to comment #5)
> (In reply to comment #3)
> >remove it from Xpfe
> IIRC it was timeless's idea so unless he can convince the Sea Lords...

<http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/toolkit/content/widgets/textbox.xml>
{{
1.10	blakeross%telocity.com	2003-08-27 01:24	 	remove "redo" item that we got
from mozilla.
}}

Blake, Josh:
Could comment on why 'redo' had/has to be removed from textbox context menu
(while 'undo' is kept) ? Thanks.
iirc, we removed it because its rarely used compared to Undo and not especially
useful in most cases.  I see no reason to re-add it to toolkit, so seamonkey has
to make the call.
Attachment #178081 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #178081 - Flags: review?(mconnor)
Attachment #176804 - Attachment is obsolete: true
Attachment #178081 - Attachment is obsolete: true
Attachment #178157 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #178157 - Flags: review?(mconnor)
Dtd cleanup check:
<http://lxr.mozilla.org/mozilla/search?string=textcontext.dtd>
{{
textcontext.dtd
/netwerk/test/jarlist.dat, line 155 --
jar:resource:///chrome/en-US.jar!/locale/en-US/global/textcontext.dtd
/toolkit/content/widgets/textbox.xml, line 4 -- <!ENTITY % textcontextDTD SYSTEM
"chrome://global/locale/textcontext.dtd" >
/toolkit/locales/jar.mn, line 44 -- + locale/@AB_CD@/global/textcontext.dtd
(%chrome/global/textcontext.dtd)
/xpfe/global/resources/content/bindings/textbox.xml, line 4 -- <!ENTITY %
textcontextDTD SYSTEM "chrome://global/locale/textcontext.dtd" >
/xpfe/global/jar.mn, line 86 -- locale/en-US/global/textcontext.dtd
(resources/locale/en-US/textcontext.dtd) 
}}
Status: NEW → ASSIGNED
Keywords: helpwanted
Target Milestone: --- → mozilla1.8beta2
Attachment #178157 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Attachment #178157 - Flags: review?(mconnor) → review+
Whiteboard: [checkin-needed]
Attachment #178157 - Flags: approval1.8b2?
Attachment #178157 - Flags: approval-aviary1.1a?
Attachment #178157 - Flags: approval1.8b2?
Attachment #178157 - Flags: approval1.8b2-
Attachment #178157 - Flags: approval-aviary1.1a?
Attachment #178157 - Flags: approval-aviary1.1a-
Comment on attachment 178157 [details] [diff] [review]
(Av1b) <textbox.xml> (full sync ++)
[Checked in: Comment 13]

'approval1.8b3=?':
Simple XUL code sync + DTD cleanup, no risk.
Attachment #178157 - Flags: approval1.8b3?
Attachment #178157 - Flags: approval1.8b3? → approval1.8b3+
We need to sync this with the browser's context menu...
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I strongly object dropping the redo menuitem.

(In reply to comment #8)
> iirc, we removed it because its rarely used compared to Undo and not
> especially useful in most cases.

Says who? This argument was already pretty stupid in bug 172047 and hasn't
become any better meanwhile. With the same reasons we'd have to remove the
forward button in the browser...
Attachment #178157 - Attachment description: (Av1b) <textbox.xml> (full sync ++) → (Av1b) <textbox.xml> (full sync ++) [Checked in: Comment 13]
Attachment #178157 - Attachment is obsolete: true
(In reply to comment #12)
> We need to sync this with the browser's context menu...

Neil,
Could you give (me) more details about what you meant ? Thanks.


(In reply to comment #14)
> I strongly object dropping the redo menuitem.

Karsten "Mnyromyr",
As you read, I already tried (and "failed") to raise this point.
Now, I would suggest to open a new bug like "add back Redo in both/common aviary
and seamonkey Toolkit" ... or forget about it :-/

(Couldn't this item be yet another preference/whatever, defaulted to: aviary
off, seamonkey on ?? I'm not sure that would be wanted ?)
Whiteboard: [checkin-needed]
Target Milestone: mozilla1.8beta2 → mozilla1.8beta3
I've rarely/never heard a complaint about this being removed from our context
menu.  We're not adding it back to toolkit, and adding yet another pref is
something else I won't sign off on.
(In reply to comment #16)
> I've rarely/never heard a complaint about this being removed from our context
> menu.

Then I wonder where bug 172047 came from?
Windows isn't everything - that many Windows users don't miss it because they're
used to their OS' deficiencies can't be an argument.

> We're not adding it back to toolkit

This is exactly what the difference between XPFE and Toolkit /made/: in case of
need, a function was just there... And it's quite strange that such basic
function is fought against - the impact upon the menu size is just minimal, but
its usefulness is tremendous!
Needing yet another extension to regain basic feature is just horrible...

> and adding yet another pref is something else I won't sign off on.

Hey, that I didn't request. ;-)
By which, of course, you mean "If the patch was good, the feature was added." ;)

FWIW, the bug you cited was filed (wrongly, IMO) as a symmetry issue, it was in
the Edit menu but not the context menu.  Which is fine if you think that context
menus should match the window menu, but the point of a context menu is to put
the most commonly-used commands in a small menu, not to mirror an existing menu...
Attachment #178157 - Attachment is obsolete: false
Depends on: 239373
Depends on: 302050
A few space & quote nits.
Attachment #210747 - Flags: superreview?(neil)
Attachment #210747 - Flags: review?(neil)
Attachment #210747 - Flags: superreview?(neil)
Attachment #210747 - Flags: superreview+
Attachment #210747 - Flags: review?(neil)
Attachment #210747 - Flags: review+
Attachment #178157 - Attachment is obsolete: true
Comment on attachment 210747 [details] [diff] [review]
(Bv1) nits, in the meantime
[Checked in: Comment 20 & 21]


Check in: { 2006-02-05 07:16	gavin%gavinsharp.com 	mozilla/xpfe/global/resources/content/bindings/textbox.xml 	1.37 }
Attachment #210747 - Attachment description: (Bv1) nits, in the meantime → (Bv1) nits, in the meantime [Checked in: Comment 20]
Attachment #210747 - Attachment is obsolete: true
Comment on attachment 210747 [details] [diff] [review]
(Bv1) nits, in the meantime
[Checked in: Comment 20 & 21]

This patch got checked in in two pieces...

Check in: { 2006-02-05 11:22	bugzilla%standard8.demon.co.uk 	mozilla/xpfe/global/resources/content/bindings/textbox.xml 	1.38 }
Attachment #210747 - Attachment description: (Bv1) nits, in the meantime [Checked in: Comment 20] → (Bv1) nits, in the meantime [Checked in: Comment 20 & 21]
Status: RESOLVED → REOPENED
Keywords: helpwanted
Resolution: FIXED → ---
Whiteboard: [SergeG: helpwanted to port bug 302050]
SeaMonkey is using toolkit now, is there anything we still need to port from xpfe textbox.xml to toolkit or can we close this bug?
IIRC XPFE textbox.xml has nothing new; since it doesn't have the inline spellchecking code, it does't have the bugs in it either ;-)
in this case, I'm re-marking the bug as fixed (due to it containing fixes that have been checked in). Please reopen or file new bugs if we run across something unexpected we still would need to port to toolkit.
Status: REOPENED → RESOLVED
Closed: 17 years ago15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.