Last Comment Bug 659248 - Increase pseudo-element enum storage from 4 bits to 5 bits
: Increase pseudo-element enum storage from 4 bits to 5 bits
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla7
Assigned To: Yoan TEBOUL
: Jet Villegas (:jet)
Depends on:
Blocks: 657953
  Show dependency treegraph
Reported: 2011-05-24 02:26 PDT by Vincent LAMOTTE
Modified: 2011-08-26 08:05 PDT (History)
7 users (show)
mounir: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch_v1.0 (1.67 KB, patch)
2011-05-24 10:28 PDT, Yoan TEBOUL
bzbarsky: review-
Details | Diff | Splinter Review
patch_v1.1 (1.67 KB, patch)
2011-05-25 03:50 PDT, Yoan TEBOUL
dbaron: review+
Details | Diff | Splinter Review

Description User image Vincent LAMOTTE 2011-05-24 02:26:50 PDT
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)


Reproducible: Always
Comment 1 User image Boris Zbarsky [:bz] (still a bit busy) 2011-05-24 07:26:18 PDT
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.
Comment 2 User image Yoan TEBOUL 2011-05-24 10:28:47 PDT
Created attachment 534824 [details] [diff] [review]

Changed NS_STYLE_INHERIT_MASK to 0x007fffff and adjusted the other masks.
Comment 3 User image Boris Zbarsky [:bz] (still a bit busy) 2011-05-24 12:03:15 PDT
NS_STYLE_HAS_TEXT_DECORATION_LINES seems to be pretty wrong with that patch.
Comment 4 User image Boris Zbarsky [:bz] (still a bit busy) 2011-05-24 12:03:55 PDT
Comment on attachment 534824 [details] [diff] [review]

That said, since I suggested the approach better to have dbaron review.
Comment 5 User image Yoan TEBOUL 2011-05-25 03:50:02 PDT
Created attachment 535017 [details] [diff] [review]

Sorry for this stupid mistake...
Comment 6 User image David Baron :dbaron: ⌚️UTC-8 2011-05-25 15:28:44 PDT
Comment on attachment 535017 [details] [diff] [review]

Comment 7 User image Mounir Lamouri (:mounir) 2011-05-26 08:50:51 PDT
This is ready to be pushed, isn't it?
Comment 8 User image Mounir Lamouri (:mounir) 2011-05-27 03:54:14 PDT
This patch has been pushed to cedar repository.
Comment 9 User image Mounir Lamouri (:mounir) 2011-05-27 08:26:33 PDT
Comment 10 User image Virgil Dicu [:virgil] [QA] 2011-08-26 00:50:30 PDT

Could anyone provide a test case or some STR to verify this issue?
Comment 11 User image Mounir Lamouri (:mounir) 2011-08-26 00:57:41 PDT
There is no need to verify this.
Comment 12 User image Boris Zbarsky [:bz] (still a bit busy) 2011-08-26 08:05:13 PDT
This change does not affect user-visible behavior.

It does allow new pseudo-elements to be added.

Note You need to log in before you can comment on or make changes to this bug.