Closed
Bug 74946
Opened 23 years ago
Closed 23 years ago
bidi: more changes and files for layout directory
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
VERIFIED
FIXED
mozilla0.9
People
(Reporter: erik, Assigned: erik)
Details
Attachments
(4 files)
This bug report will be used to attach some new files and diffs from the IBM bidi project. They are all under the mozilla/layout directory. Here are the new files: ? base/public/nsITextFrame.h ? html/base/src/nsBidiFrames.cpp ? html/base/src/nsBidiFrames.h And here are the files that are being modified: M base/src/nsBidiPresUtils.cpp M html/base/src/nsLineLayout.cpp M html/base/src/nsTextFrame.cpp Marc, nsITextFrame is the new interface that we discussed a while ago. This cleans up the somewhat odd subclassing that we reviewed earlier.
Assignee | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
Assignee | ||
Comment 3•23 years ago
|
||
Assignee | ||
Comment 4•23 years ago
|
||
Comment 5•23 years ago
|
||
I'd suggest changing the use of 'long' to PRInt32 - I'm not used to seeing non-abstracted types in Layout code! Also, the block that is commented as a non-BIDI change should either be proven to be incorrect and thus just fixed (eg. not in an #ifedf IBMBIDI) or have a bug opened on it so we can figure out if it is wrong and fix it, or correct and clarify why it needs to be the way it is. I'll do that if you like. sr=attinasi
Assignee | ||
Comment 6•23 years ago
|
||
Just to make sure we are talking about the same thing, Marc, are you referring to the following block of code? if (inContentOffset < mContentOffset) //could happen with floaters! { result = GetPrevInFlow(outChildFrame); +#ifdef IBMBIDI // Not a Bidi change; I just thought this line was wrong + if (NS_SUCCEEDED(result) && *outChildFrame) +#else if (NS_SUCCEEDED(result) && outChildFrame) +#endif return (*outChildFrame)->GetChildFrameContainingOffset(inContentOffset, inHint, outFrameContentOffset,outChildFrame);
Comment 7•23 years ago
|
||
Erik, yes that is the code I was referring to - thanks for checking. Looking at the full source just now, I cannot see how the old version could be possibly be correct, so I'd be inclined to just change it with no #ifdef's. Note that outChildFrame is a **, and every place else that GetPrevInFlow is used, the check is on the value returned from the method, not on the address of the returned value (using the word 'return' lightly here to mean an output argument).
Assignee | ||
Comment 9•23 years ago
|
||
r=erik@netscape.com
Assignee | ||
Comment 10•23 years ago
|
||
New files and fixes checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 11•23 years ago
|
||
Marc: 'long' has the same size as 'void*', while using PRInt32 may be problematical for a 64-bit platform.
Assignee | ||
Comment 12•23 years ago
|
||
Lina, what size are the things that we are storing inside the void*? Are they 8-bit? In that case PRInt32 or even PRUint8 should be OK, no?
Comment 13•23 years ago
|
||
Erik, we'are storing also 'void*' there. Cannot we replace 'long' by 'PRWord' (defined in prtypes.h)? (By the way, I'm talking here about the type of the last argument of GetBidiProperty(); as for the callers of this method, they could safely use 'PRInt32', indeed, but then need to specify explicitly the size of this type - sizeof(PRInt32).)
Assignee | ||
Comment 14•23 years ago
|
||
PRWord isn't a good choice either, since it isn't used much in the code base either. I think we should just use whatever type those things are supposed to be, e.g. nsBidiLevel, then pass sizeof(nsBidiLevel) to GetBidiProperty().
Comment 15•23 years ago
|
||
code landing bug. change QA to ftang v=ftang
Status: RESOLVED → VERIFIED
QA Contact: petersen → ftang
You need to log in
before you can comment on or make changes to this bug.
Description
•