Closed Bug 1441729 Opened 2 years ago Closed 2 years ago

Remove aTextIsSignificant param from nsStyleUtil::IsSignificantChild and its friends

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

Details

Attachments

(1 file)

All callsites of this function pass true for it, so it makes no sense to keep it at all.

Reading back the history, the third param aWhitespaceIsSignificant was introduced in bug 157395. From the code, it can be seen that at that age, IsSignificantChild is also used by :{only,first,last}-child pseudo-class selectors, which passes false for aTextIsSignificant. [1]

We know that we have been using some more complicated mechanism for the tree-structural pseudo-classes, and aTextIsSignificant param lost its usefulness since then.

So we can remove it now.


[1] https://github.com/mozilla/gecko-dev/commit/6de447be6a438f24d5209e7296908876a200cd53
Given this history, we can probably even rename IsSignificantChild to something like IsNodeEmpty or something like that, and probably have an enum for that...
Comment on attachment 8954628 [details]
Bug 1441729 - Remove aTextIsSignificant param from nsStyleUtil::IsSignificantChild and its friends.

https://reviewboard.mozilla.org/r/223702/#review229804

Nice catch :)
Attachment #8954628 - Flags: review?(emilio) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1fcc8099f2d9
Remove aTextIsSignificant param from nsStyleUtil::IsSignificantChild and its friends. r=emilio
https://hg.mozilla.org/mozilla-central/rev/1fcc8099f2d9
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.