The default bug view has changed. See this FAQ.

Increase pseudo-element enum storage from 4 bits to 5 bits

RESOLVED FIXED in mozilla7

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Vincent LAMOTTE, Assigned: Yoan TEBOUL)

Tracking

Trunk
mozilla7
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: 

Adding a new CSS Pseudo-Element for <meter> made this assert to fault during compilation : (nsStyleContext.cpp, line 79)

PR_STATIC_ASSERT((PR_UINT32_MAX >> NS_STYLE_CONTEXT_TYPE_SHIFT) >=
                  nsCSSPseudoElements::ePseudo_MAX);
 


Reproducible: Always
(Reporter)

Updated

6 years ago
Blocks: 657953
Keywords: css-moz
Version: unspecified → Trunk
Status: UNCONFIRMED → NEW
Component: DOM: CSS Object Model → Style System (CSS)
Ever confirmed: true
Keywords: css-moz
Uh... this is not a 32-bit vs 64-bit issue.

Right now nsStyleContext stores the pseudo-element type in mBits.  It also stores the NS_STYLE_INHERIT_MASK and some additional boolean bits there.  That leaves 4 bits for the pseudo-element type.  So your pseudo-element enum needs to fit in 4 bits right now.

I believe that we currently have 23 style structs but NS_STYLE_INHERIT_MASK is 0x00ffffff.  So what you could do is change NS_STYLE_INHERIT_MASK to 0x008fffff (if this is not safe the static assert in nsStyleStruct.cpp will tell you) and then shift down by one the 4 style context boolean bits and change NS_STYLE_CONTEXT_TYPE_SHIFT to 27.  That will give you 5 bits for the pseudo-element type, which should be good enough for now.
(Assignee)

Comment 2

6 years ago
Created attachment 534824 [details] [diff] [review]
patch_v1.0

Changed NS_STYLE_INHERIT_MASK to 0x007fffff and adjusted the other masks.
(Assignee)

Updated

6 years ago
Attachment #534824 - Flags: review?(bzbarsky)
NS_STYLE_HAS_TEXT_DECORATION_LINES seems to be pretty wrong with that patch.
Comment on attachment 534824 [details] [diff] [review]
patch_v1.0

That said, since I suggested the approach better to have dbaron review.
Attachment #534824 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 5

6 years ago
Created attachment 535017 [details] [diff] [review]
patch_v1.1

Sorry for this stupid mistake...
Attachment #535017 - Flags: review?(dbaron)
Summary: Increase pseudo-element count from a 32bit length int to a 64bit length → Increase pseudo-element enum storage from 4 bits to 5 bits
Comment on attachment 535017 [details] [diff] [review]
patch_v1.1

r=dbaron
Attachment #535017 - Flags: review?(dbaron) → review+
Assignee: nobody → yoan.teboul
Status: NEW → ASSIGNED
This is ready to be pushed, isn't it?
Flags: in-testsuite-
Whiteboard: ced
This patch has been pushed to cedar repository.
Whiteboard: ced → [fixed in cedar]
Pushed:
http://hg.mozilla.org/mozilla-central/rev/ef6562c76cee
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Target Milestone: --- → mozilla7
Hello.

Could anyone provide a test case or some STR to verify this issue?
There is no need to verify this.
This change does not affect user-visible behavior.

It does allow new pseudo-elements to be added.
You need to log in before you can comment on or make changes to this bug.