Priority: -- → P1
Target Milestone: --- → mozilla1.8beta
Created attachment 179732 [details] [diff] [review] Proposed patch Add some layout utils methods to hide the casting. Note that I didn't just put them in the header to avoid having to include nsIFrame.h and nsPlaceholderFrame.h (the latter not being exported, I'd have had to munge LOCAL_INCLUDES in places that include nsLayoutUtils.h). I think this should build fine as long as callers that use the methods I added include nsPlaceholderFrame.h and nsIFrame.h themselves (and I did test that this builds with gcc). The other option, if this fails to compile, is to make these methods _IMPL_NS_LAYOUT, as dbaron suggested, and hope that all places that define that also have the right dirs in LOCAL_INCLUDES.... or just change LOCAL_INCLUDES appropriately. I really do want to have these be inline, though, since I'm basically just replacing a cast with one of the methods in question.
Summary: Create nsLayoutUtils::GetRealFrameFor → [FIX]Create nsLayoutUtils::GetRealFrameFor
Target Milestone: mozilla1.8beta1 → mozilla1.8beta2
Are you sure those are getting inlined? I would think not.
Not completely sure, no. How would I tell? I can go the _IMPL_NS_LAYOUT route (and put the bodies in the header too) instead if you prefer...
So it looks like various content code defines _IMPL_NS_LAYOUT.... Should I just add layout/generic to the relevant LOCAL_INCLUDES, or export nsPlaceholderFrame.h? I guess the former is better, eh?
Another option is to just define them as static members of nsPlaceholderFrame.
Hmm... Yeah, that sounds like a reasonable idea.
Created attachment 180018 [details] [diff] [review] With that change
Summary: [FIX]Create nsLayoutUtils::GetRealFrameFor → [FIXr]Create nsLayoutUtils::GetRealFrameFor
Target Milestone: mozilla1.8beta2 → mozilla1.8beta3
Comment on attachment 180018 [details] [diff] [review] With that change Requesting 1.8 approval. This is a very safe change; just makes us not copy this casting code all over.
Attachment #180018 - Flags: approval1.8b2?
Comment on attachment 180018 [details] [diff] [review] With that change a=brendan for 1.8b2. /be
Attachment #180018 - Flags: approval1.8b2? → approval1.8b2+
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.8beta3 → mozilla1.8beta2
Some of these calls need null-checking: ###!!! ASSERTION: Must have a frame to work with: 'aFrame', file ../generic/nsPlaceholderFrame.h, line 99 which in my case was called from nsFocusIterator.cpp line 564
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 180812 [details] [diff] [review] Fix for Neil's crash All the other places already null-checked; dunno how I missed this one. :(
Checked in the crash fix.
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago → 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.