User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.12) Gecko/20051114 Firefox/1.0.7 (Debian package 1.0.7-1) Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9a1) Gecko/20051130 Firefox/1.6a1 The patch submitted here does all the work necessary to completely remove the 'nsTextFrame.h' file. This means the entire public interface for nsTextFrame.cpp is reduced to just nsIFrame* NS_NewTextFrame(nsIPresShell* aPresShell); nsIFrame* NS_NewContinuingTextFrame(nsIPresShell* aPresShell); (defined in nsHTMLParts.h) This will allow a new version of nsTextFrame specific to the new Thebes toolkit to be developed in a different .cpp file, switchable by a conditional in the Makefile. This approach was chosen because the work to be done on nsTextFrame will probably be major, and it keeps things far tidier than the alternative of filling nsTextFrame with #ifdefs. This patch moves two functions that do not need to be defined in nsTextFrame.h: 1. BinarySearchForPosition moved to nsLayoutUtils. 2. HasTerminalNewline moved to nsIFrame.h and nsFrame.cpp. NOTE!! This patch doesn't actually remove layout/generic/nsTextFrame.h, so this must be done in addition to applying this patch. Reproducible: Always Steps to Reproduce:
Created attachment 205015 [details] [diff] [review] Code changes necessary to remove layout/generic/nsTextFrame.h
checked in. Thanks. Have fun!
Status: UNCONFIRMED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Is it a good idea to duplicate the comments in the header and the implementation? that's twice as much text that can become oudated...
No, you're right
13 years ago
Component: General → Layout: Fonts and Text
Product: Firefox → Core
QA Contact: general → layout.fonts-and-text
Version: unspecified → Trunk
Huh, I just noticed this bug. I suppose it answers BZ's question in the second paragraph of bug 306895 comment #6, where he encouraged me *not* to declare HasTerminalNewline in nsIFrame, but rather directly in nsTextFrame (and to move the nsTextFrame declaration from nsTextFrame.cpp into nsTextFrame.h - see bug 306895 comment #8). It seems that now we're going in the reverse direction... I just want to make sure everybody is on the same page here.
I'm OK with this on nsIFrame if that's a temporary thing, I guess. Please file a followup bug to move it back off nsIFrame when you're done with the Thebes business? And make sure that's fixed before we ship our next Gecko?
13 years ago
Depends on: 321570
You need to log in before you can comment on or make changes to this bug.