The default bug view has changed. See this FAQ.

XPFE autocomplete.xml: investigate removing explicit <children includes="menupopup"/>

RESOLVED FIXED in Firefox 6

Status

()

Toolkit
XUL Widgets
--
minor
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Trunk
mozilla6
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox6 fixed)

Details

(Whiteboard: [fixed in moz5: Av1; moz6: Bv1], URL)

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
The goal would be to have the XPFE children in the same order as the Toolkit children, to reduce differences.

Bug 535893 comment 11:
{
neil@parkwaycc.co.uk      2011-03-17 03:27:35 PDT

The XPFE autocomplete has an extra explicit <children includes="menupopup"/>
entry in its XBL but toolkit seems to manage perfectly well without it. Maybe
worth a followup bug to investigate removing it.
}

Code is:
9   <binding id="autocomplete"
16     <content sizetopopup="pref">
40       <children includes="menupopup"/>
41     </content>
(Assignee)

Comment 1

6 years ago
Ftr,

This line was added in XPFE in:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/xpfe/components/autocomplete/resources/content/autocomplete.xml&rev=1.33
1.33	hewitt%netscape.com	2001-04-30 11:36	 	43189 - autocomplete, r=matt, sr=alecf

It looks like Toolkit never had this line:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/toolkit/content/widgets/autocomplete.xml&rev=1.1
1.1	hewitt%netscape.com	2002-09-27 23:21	 	165955 - turning on satchel in phoenix
(Assignee)

Comment 2

6 years ago
Created attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]
Attachment #519908 - Flags: review?(neil)

Comment 3

6 years ago
Comment on attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]

Needs a11y review too right?
Attachment #519908 - Flags: review?(neil) → review+
(Assignee)

Comment 4

6 years ago
Comment on attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]

(In reply to comment #3)
> Needs a11y review too right?

I don't think so in this case, but I'll ask for feedback (at least).
Attachment #519908 - Flags: feedback?(surkov.alexander)

Comment 5

6 years ago
Comment on attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]

feedback is ok here, but yes - either review or feedback from a11y peer is required when a11y mochitest are changed.
Attachment #519908 - Flags: feedback?(surkov.alexander) → feedback+
(Assignee)

Comment 6

6 years ago
Comment on attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]

http://hg.mozilla.org/mozilla-central/rev/ddd48e3b173a
Attachment #519908 - Attachment description: (Av1) Just remove it, Update a11y test_combobox.xul → (Av1) Just remove it, Update a11y test_combobox.xul [Checked in: Comment 6]
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Summary: XPFE autocomplete.xml: investigate removing explicit <children includes="menupopup"/>. → XPFE autocomplete.xml: investigate removing explicit <children includes="menupopup"/>
Target Milestone: --- → mozilla2.0
(Assignee)

Updated

6 years ago
Target Milestone: mozilla2.0 → mozilla2.2
(Assignee)

Comment 7

6 years ago
Comment on attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]

"approval2.0=?":
NPOTDB, FF2SM sync'.
Attachment #519908 - Flags: approval2.0?
Comment on attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]

I didn't understand the rationale in that last comment for why this must land on the mozilla2.0 repository
(Assignee)

Comment 9

6 years ago
(In reply to comment #8)
> I didn't understand the rationale in that last comment for why this must land
> on the mozilla2.0 repository

So upcoming SeaMonkey 2.1 can have this fix. (Firefox is unaffected.)
(Assignee)

Comment 10

6 years ago
Ping for approval.
Comment on attachment 519908 [details] [diff] [review]
(Av1) Just remove it, Update a11y test_combobox.xul
[Checked in: Comment 6]

not more planned 4.0.x releases, minused
Attachment #519908 - Flags: approval2.0? → approval2.0-
This caused bug 665538.
Status: RESOLVED → REOPENED
Depends on: 665538
Resolution: FIXED → ---
(Assignee)

Comment 13

6 years ago
In order to back this out, at least ftb,
Would it be possible to move |<children includes="menupopup"/>| around to still get the same child order as Toolkit has?
What comment could be added to explain a little why that code is needed (wrt bug 665538)?
(In reply to comment #13)
> Would it be possible to move |<children includes="menupopup"/>| around to
> still get the same child order as Toolkit has?
Possibly. Try putting it at the start of the binding.
(Assignee)

Comment 15

6 years ago
Created attachment 543709 [details] [diff] [review]
(Bv1) XPFE autocomplete.xml: restore explicit <children includes="menupopup"/>, but at start instead of at end
[Checked in: Comment 17 & 20]

Tested (with an old "2.1.1pre" build, as newer omni.jar can't be opened/edited).

"approval-mozilla-aurora=?" and "approval-mozilla-beta=?":
Unregress SeaMonkey v2.2+ from patch Av1. NPOfirefoxB, no risk.
Attachment #543709 - Flags: review?(neil)
Attachment #543709 - Flags: approval-mozilla-beta?
Attachment #543709 - Flags: approval-mozilla-aurora?

Comment 16

6 years ago
I've applied the patch to a SM 2.2b3 Win32 build and it fixes bug 665538 for me.

Updated

6 years ago
Attachment #543709 - Flags: review?(neil) → review+
(Assignee)

Comment 17

6 years ago
Comment on attachment 543709 [details] [diff] [review]
(Bv1) XPFE autocomplete.xml: restore explicit <children includes="menupopup"/>, but at start instead of at end
[Checked in: Comment 17 & 20]

http://hg.mozilla.org/mozilla-central/rev/29bc151be813
Attachment #543709 - Attachment description: (Bv1) XPFE autocomplete.xml: restore explicit <children includes="menupopup"/>, but at start instead of at end → (Bv1) XPFE autocomplete.xml: restore explicit <children includes="menupopup"/>, but at start instead of at end [Checked in: Comment 17]
(Assignee)

Updated

6 years ago
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [SeaMonkey v2.2-v2.3 are not unregressed (yet)]
Please, when pushing to central:
1. Care about your push: star oranges asap, retrigger broken builds.
2. Be available in #developers.

Otherwise, we created the mozilla-inbound repository for whoever doesn't have the time or will to follow his own pushes. So just push in mozilla-inbound and people will take care of doing that for you.

Putting more work on sheriff's shoulders is not going to help anyone.
Thanks.
Or better still, mark NPOMCB patches with DONTBUILD in the checkin comment.

Updated

6 years ago
Attachment #543709 - Flags: approval-mozilla-beta?
Attachment #543709 - Flags: approval-mozilla-beta+
Attachment #543709 - Flags: approval-mozilla-aurora?
Attachment #543709 - Flags: approval-mozilla-aurora+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: [SeaMonkey v2.2-v2.3 are not unregressed (yet)] → [c-n: Bv1 to m-a & m-b] [SeaMonkey v2.2-v2.3 are not unregressed (yet)]

Comment 20

6 years ago
http://hg.mozilla.org/releases/mozilla-beta/rev/a49dce91dc3c
status-firefox6: --- → fixed

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: [c-n: Bv1 to m-a & m-b] [SeaMonkey v2.2-v2.3 are not unregressed (yet)]
(Assignee)

Updated

6 years ago
Attachment #543709 - Attachment description: (Bv1) XPFE autocomplete.xml: restore explicit <children includes="menupopup"/>, but at start instead of at end [Checked in: Comment 17] → (Bv1) XPFE autocomplete.xml: restore explicit <children includes="menupopup"/>, but at start instead of at end [Checked in: Comment 17 & 20]
(Assignee)

Comment 21

6 years ago
(In reply to Marco Bonardo [:mak] from comment #18)
> Otherwise, we created the mozilla-inbound repository for whoever doesn't
> have the time or will to follow his own pushes.

I wasn't familiar with mozilla-inbound.
I will try to use it, or just use checkin-needed.


(In reply to neil@parkwaycc.co.uk from comment #19)
> Or better still, mark NPOMCB patches with DONTBUILD in the checkin comment.

Right. I did it for patch Av1, but I did miss this for patch Bv1 :-<
(Assignee)

Updated

5 years ago
Whiteboard: [fixed in moz5: Av1; moz6: Bv1]
Target Milestone: mozilla5 → mozilla6
(Assignee)

Comment 22

5 years ago
Comment on attachment 543709 [details] [diff] [review]
(Bv1) XPFE autocomplete.xml: restore explicit <children includes="menupopup"/>, but at start instead of at end
[Checked in: Comment 17 & 20]

Ftr, this unregression patch never made it to mozilla5.
You need to log in before you can comment on or make changes to this bug.