This was discovered by our static-checking suite (specifically layout/generic/frame-verify.js). Seems like http://hg.mozilla.org/mozilla-central/file/2e35085ca7cc/layout/base/nsBidiPresUtils.cpp#l212 is rarely executed if ever?
Attachment #491758 - Flags: review?(roc) → review+
Assignee: nobody → ehren.m
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
The block of code should be executed by our crashtests at least.
backed out because of failing reftest/tests/layout/base/crashtests/436982-1.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Actually, it won't let me push the backout right now for whatever reason...
Ok, pushed finally.
(In reply to comment #0) > Seems like > http://hg.mozilla.org/mozilla-central/file/2e35085ca7cc/layout/base/nsBidiPresUtils.cpp#l212 > is rarely executed if ever? More likely the GetType above means that it ends up doing the "right" thing.
So is the crash because the patch doesn't have all the queryframe macros it needs?
I think this should solve the issue. Patch v1 is definitely incorrect and results in nsnull returned by nsQueryFrame (it eventually defaults to NS_QUERYFRAME_TAIL_INHERITANCE_ROOT). Without the patch the behavior is correct but only because nsQueryFrame eventually defaults to NS_QUERYFRAME_TAIL_INHERITING (I'd have to look at the pre-processed source to really understand this though). In any case, replacing the do_QueryFrame call with a static_cast gives me the same behavior as the unpatched code when stepping through the subsequent call to CreateContinuationForFloatingParent (a normal cast is basically what QueryFrame ends up doing anyway). I've got a try server and static analysis build going now though so I'll wait till they complete before requesting review (should have done this the first time!).
Attachment #491758 - Attachment is obsolete: true
We should probably just do queryframe correctly. I think you can follow the pattern of nsFrameSetFrame for that.
followed the pattern of nsHTMLFrameSetFrame.
Attachment #493527 - Flags: review?(tnikkel) → review?(roc)
Attachment #493527 - Flags: review?(roc) → review+
Comment on attachment 493527 [details] [diff] [review] patch v3 I think we should hold off on this and land it after we branch for Firefox 4 / Gecko 2.
Attachment #493527 - Flags: approval2.0? → approval2.0-
Comment on attachment 491758 [details] [diff] [review] patch (bookkeeping)
Attachment #491758 - Flags: approval2.0+
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago → 8 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.