1031149 - nsStyleContext::FindChildWithRules cache is passed an incorrect aRelevantLinkVisited parameter

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

nsStyleSet::GetContext passes nsStyleContext::FindChildWithRules an incorrect aRelevantLinkVisited parameter.  This parameter should correspond to whether RelevantLinkVisited() will be set on the style context, since that's what FindChildWithRules assumes it means for the cache lookup.  However, GetContext hasn't yet completed the computation of whether that will be set on the style context, so it can be incorrect for descendants of links.  Some of GetContext's callers fix the value up correctly, but the primary one (ResolveStyleFor) doesn't.  See patch to be attached.  This might lead to some styling confusion.

I should investigate whether this fixes any of:  bug 554630, bug 557579, bug 575675, bug 618792, bug 868975, bug 931627, bug 952328.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://tbpl.mozilla.org/?tree=Try&rev=36be24dabdd9

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://tbpl.mozilla.org/?tree=Try&rev=79281705b295

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Consistently pass correct aRelevantLinkVisited to nsStyleContext::FindChildWithRules

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8447632 [details] [diff] [review]
Consistently pass correct aRelevantLinkVisited to nsStyleContext::FindChildWithRules

Review of attachment 8447632 [details] [diff] [review]:
-----------------------------------------------------------------

Should we move the

  if (aOldStyle->RelevantLinkVisited()) {
    flags |= eIsVisitedLink;
  }

block in nsStyleSet::ResolveStyleForRules into the

  if (aOldStyle->IsLinkContext())

block?

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Bug 996796 patch 9 removes that function entirely (and later patches in that bug fix various bugs in it), so I'd rather not mess with it.

Comment 6

[Cameron McCormack (:heycam)]

Fair enough.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And, er, actually, bug 996796 patch 11 does exactly that, in the moved code.  (I wrote this patch while working on that part of the patch series there.)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fc64f044901

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9fc64f044901