timing issues in the outliner content model.

VERIFIED FIXED in mozilla1.0

Status

()

Core
XUL
--
major
VERIFIED FIXED
16 years ago
6 years ago

People

(Reporter: Suresh, Assigned: janv)

Tracking

Trunk
mozilla1.0
x86
Windows 2000
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ADT1] [m5+][hitlist] [ETA 04/22] [Needs a=] [landed on the trunk], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
Looks like there is some timing issues in the content model when we load the
data from the datasource. Hewitt saw this problem when we were debugging this in
the commercial tree.

Let me know if you folks need a test case to work on. Thanks!
(Reporter)

Comment 1

16 years ago
Nominating. Commercial tree has seen this behaviour lately! 
Keywords: nsbeta1
(Assignee)

Comment 2

16 years ago
sure, a testcase whould be nice

Comment 3

16 years ago
What is the end user impact of this bug?  What are the prominent consumers of
the outliner using a datasource to populate themselves?  How rampant is this
problem?  Does it only occur on slower machines?  Thanks.

Comment 4

16 years ago
See bugscape bug 13003 for impact.

Updated

16 years ago
Severity: normal → major
(Reporter)

Comment 5

16 years ago
varga, The one testcase I had in my mind works fine now. But it doesn't work in
one of the commercial module :(  Hewitt knows/saw the problem. Prolly he can
detail you. Thanks!
(Reporter)

Comment 6

16 years ago
To answer samir's questions:

1. (as jelwell mentioned) see bugscape 13003 and 13191 for impact.
2. This problem occurs in our own product.
3. I have seen this problem in my 1 Ghz system. 

Comment 7

16 years ago
nsbeta1- per Nav triage team.  Both of the cited bugs are fixed, so if there is
still some serious problem, please describe it and the impact on users here.
Keywords: nsbeta1 → nsbeta1-
Target Milestone: --- → mozilla1.1alpha

Comment 8

16 years ago
New info is that this also depends on
http://bugscape.mcom.com/show_bug.cgi?id=13670, which is nsbeta1+/ADT1.  Marking
nsbeta1+/ADT1.  Jan, do you have time to help on this?
Keywords: nsbeta1- → nsbeta1+
Whiteboard: ADT1
(Reporter)

Comment 9

16 years ago
Bugscape 13003 is marked invalid because we choose to ship with something
similar but different one. 

http://bugscape.netscape.com/show_bug.cgi?id=13670 deals with the new one.
(Assignee)

Comment 10

16 years ago
Sure, could someone send me a private copy of the bugscape bug ?
A testcase demonstrating this problem would be great.

Comment 11

16 years ago
This is a pretty important bug [m5+], shouldn't we try and get it into 1.0?
Whiteboard: ADT1 → [ADT1] [m5+]

Updated

16 years ago
Target Milestone: mozilla1.1alpha → mozilla1.0

Comment 12

16 years ago
adding metabug blockage
Blocks: 136384

Comment 13

16 years ago
I don't really understand what is going on here, either.  I can only describe
the symptoms.  In the buddy list, sometimes certain rows will get appended at
the wrong level and row index in the view.  The buddy list uses the content
view, and is generated by an rdf template.

reassigning to Jan since he is the tree content view dude
Assignee: hewitt → varga
(Assignee)

Comment 14

16 years ago
working on a testcase

Comment 15

16 years ago
Taking this on my hitlist.

Updated

16 years ago
Whiteboard: [ADT1] [m5+] → [ADT1] [m5+][hitlist]
(Assignee)

Updated

16 years ago
Depends on: 137890
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Whiteboard: [ADT1] [m5+][hitlist] → [ADT1] [m5+][hitlist] [ETA 04/22]
(Assignee)

Comment 16

16 years ago
Created attachment 79868 [details] [diff] [review]
patch to support hidden attribute on a treeitem
(Assignee)

Comment 17

16 years ago
Created attachment 79949 [details] [diff] [review]
new patch

- fixed AttributeChanged() to only care about its own children
Attachment #79868 - Attachment is obsolete: true
Comment on attachment 79949 [details] [diff] [review]
new patch

r=bryner
Attachment #79949 - Flags: review+
(Reporter)

Comment 19

16 years ago
hewitt, can you sr this bug? Thanks.

Comment 20

16 years ago
Comment on attachment 79949 [details] [diff] [review]
new patch

sr=hewitt
Attachment #79949 - Flags: superreview+

Comment 21

16 years ago
The SR is here, let's tryn and get this one in asap. adt1.0.0.
Keywords: adt1.0.0, approval

Updated

16 years ago
Whiteboard: [ADT1] [m5+][hitlist] [ETA 04/22] → [ADT1] [m5+][hitlist] [ETA 04/22] [Needs a=]
(Assignee)

Updated

16 years ago
Whiteboard: [ADT1] [m5+][hitlist] [ETA 04/22] [Needs a=] → [ADT1] [m5+][hitlist] [ETA 04/22] [Needs a=] [landed on the trunk]

Comment 22

16 years ago
I'm asking some people to test this out in mailnews but they are saying that the
patch fails  to build on the branch.  Could you provide and updated patch?

Comment 23

16 years ago
It was actually a problem applying the patch, not building with it.

Comment 24

16 years ago
I failed too, in a way I am not comfortable manually fixing (I tried, but seems
like there was enough change between the branch and the tree this patch was
applied to that I cannot be sure). Could you apply an updated patch for the
branch so we can do some testing tonight?

Comment 25

16 years ago
Ok, I did this carefully by hand and feel good about it -- that patch I will be
posting should be easily applied to the current branch. There was one chunk of
code that appears to have been new to Jan's tree that is not in the branch. That
was what was causing the patch to fail. Jan, please look at the patch and verify
this is what we need for the branch. 

Comment 26

16 years ago
Created attachment 81258 [details] [diff] [review]
Patch that works for mozilla1.0.0 branch

This patch can be used for testing. Ultimately, Jan Varga should validate the
patch before it lands.
(Assignee)

Comment 27

16 years ago
Comment on attachment 81258 [details] [diff] [review]
Patch that works for mozilla1.0.0 branch

Yeah, this looks good.
Actually, it was patch for optgroups which made this not apply on the 1.0
branch
Attachment #81258 - Flags: review+
(Assignee)

Comment 28

16 years ago
r=varga
(Reporter)

Comment 29

16 years ago
yeah, I had problems with the when I originally tried. I ended up manually doing
myseld! 
(Reporter)

Comment 30

16 years ago
This is fixed in the trunk and the dependent bugscape bug
(http://bugscape.netscape.com/show_bug.cgi?id=13670) is verified. Marking this
as fixed so that adt can approve this to checkin to the branch. 
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 31

16 years ago
adding adt1.0.0+.  Please check this into the branch as soon as possible and add
the fixed1.0.0 keyword after getting drivers approval.
Keywords: adt1.0.0 → adt1.0.0+
(Assignee)

Comment 32

16 years ago
landed on the branch
Keywords: fixed1.0.0
No driver gave a=.  This should not have been checked into the branch.  Also,
the branch patch has no sr marked.  Please justify leaving this patch in to
drivers@mozilla.org, and also please obtain an sr= for the branch patch.

rjesup@wgate.com for drivers.
(Assignee)

Comment 34

16 years ago
I'm confused with this adt=, a= and whatever stuff
This patch is almost identical to previous one. I got first sr= after 5 days.
This patch was tested by Navigator and Mailnews QA and I was requested to check
in  to the branch.
So now I'm not sure what I was supposed to do actually.
In case that this is problem, back it out

Comment 35

16 years ago
I think this was just an honest oversight, made more likely by ADT time pressure
(deadline of 3AM!).  If there are no known problems with the checkin, could we
possibly just get SR= on the changes from the original patch, and a= from
drivers?  That would be a lot more efficient than backing out, going through the
correct Process, and checking in again.

Comment 36

16 years ago
That patch was a part of the binary that was sent around for testing. So, it is
very likely good, it was just a matter of manually applying the patch because of
some unrelated differences between trunk and branch. My thought is the sr= from
the trunk patch is sufficient for the branch one (in fact, the reason I placed
that patch here was just to make it easy on the person who had to checkin the
patch to the branch, not because it was anything significantly different).

Comment 37

16 years ago
Comment on attachment 81258 [details] [diff] [review]
Patch that works for mozilla1.0.0 branch

sr=hewitt
Attachment #81258 - Flags: superreview+
Comment on attachment 81258 [details] [diff] [review]
Patch that works for mozilla1.0.0 branch

retroactive a=rjesup@wgate.com for drivers.  Thanks for clearing up what
happened.
Attachment #81258 - Flags: approval+
(Reporter)

Comment 39

16 years ago
HUGE thanks to Varga for fixing this bug and also to everyone else involved in
this bug!!

The dependent bugscape bug is verified in branch and trunk. Marking this as
verified.
Status: RESOLVED → VERIFIED

Comment 40

16 years ago
Just to be sure we are clear of this problem some testing on slow (e.g., 200Mhz)
machines should be done in the coming weeks. I ran into some problems today with
a newer build and it is not clear to me we are completely out of the woods yet.

 

Updated

10 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: shrir → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.