Closed
Bug 960312
Opened 10 years ago
Closed 10 years ago
TEXT_IS_IN_SINGLE_CHAR_MI frame state bit should have a different name
Categories
(Core :: MathML, defect)
Core
MathML
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: heycam, Assigned: heycam)
Details
Attachments
(1 file, 2 obsolete files)
8.65 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
The TEXT_IS_IN_SINGLE_CHAR_MI frame state bit defined in nsTextFrame.h uses one of the global frame state bits rather than a frame class specific one. Note the comment in nsIFrame.h that says: // Bits 20-31 and 60-63 of the frame state are reserved for implementations. Luckily there is a single nsTextFrame-specific bit free -- NS_FRAME_STATE_BIT(30).
Attachment #8360706 -
Flags: review?(roc)
Attachment #8360706 -
Flags: review?(roc) → review+
Assignee | ||
Comment 1•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/994659cbc145
Assignee | ||
Comment 2•10 years ago
|
||
Backed out for test failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/8219df0931e3
(In reply to Cameron McCormack (:heycam) from comment #2) > Backed out for test failures: > https://hg.mozilla.org/integration/mozilla-inbound/rev/8219df0931e3 All of the failures are similar to https://tbpl.mozilla.org/php/getParsedLog.php?id=33074391&tree=Mozilla-Inbound (but are happening in at least m2, m5, C, Cipc, R, Ru... and on at least both linux and osx), the common thing seems to be crashing in [@ nsIFrame::GetWritingMode() const]
Assignee | ||
Comment 4•10 years ago
|
||
This state bit is also set on the block frame that contains the single character operator, so not just on text frames. Let's rename the constant to make it obvious it's global. (It's probably possible to avoid it being a global state bit with some effort. We've only got 4 left.)
Assignee | ||
Updated•10 years ago
|
Summary: TEXT_IS_IN_SINGLE_CHAR_MI frame state bit should have a different value → TEXT_IS_IN_SINGLE_CHAR_MI frame state bit should have a different name
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8360706 -
Attachment is obsolete: true
Attachment #8360861 -
Flags: review?(roc)
Comment on attachment 8360861 [details] [diff] [review] patch v2 Review of attachment 8360861 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsTextFrame.cpp @@ +249,5 @@ > // nsTextFrame.h has > // #define TEXT_IS_IN_TOKEN_MATHML NS_FRAME_STATE_BIT(32) > > // nsTextFrame.h has > +// #define NS_FRAME_IS_IN_SINGLE_CHAR_MI NS_FRAME_STATE_BIT(59) Why is this still here?
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) (away January 20-24) from comment #6) > > // nsTextFrame.h has > > +// #define NS_FRAME_IS_IN_SINGLE_CHAR_MI NS_FRAME_STATE_BIT(59) > > Why is this still here? It should go.
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8360861 [details] [diff] [review] patch v2 You know TEXT_IS_IN_TOKEN_MATHML should be renamed too. Lemme do that.
Attachment #8360861 -
Flags: review?(roc)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #8) > You know TEXT_IS_IN_TOKEN_MATHML should be renamed too. Lemme do that. Well, looks like there is some deliberate squatting on the global namespace bits there. That slot used to be taken up by TEXT_FORCE_TRIM_WHITESPACE which is mentioned in the comment about NS_FRAME_IS_PUSHED_FLOAT with which it shares its value.
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8360863 -
Flags: review?(roc)
Assignee | ||
Updated•10 years ago
|
Attachment #8360861 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8360863 -
Attachment is patch: true
Attachment #8360863 -
Flags: review?(roc) → review+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec669f48ec9e
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ec669f48ec9e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•