Newtab's hard coded layout has a small positioning issue in the 4-medium-small-1-ad layout
Categories
(Firefox :: New Tab Page, defect)
Tracking
()
People
(Reporter: thecount, Unassigned)
References
(Blocks 1 open bug)
Details
4-medium-small-1-ad moves the spoc around in the 4 column layout. In cols 1, 2, and 3 it's in position 1 in the second index. In the 4 column layout, it's position 3 in the 4th index. It should be position: 1 in the 4th index.
This is likely not a visible issue, but a data one. Luckily this is low in the scroll order and likely not even an ad placement half the time.
I see this in both the hard coded layout, and the RS layout. I don't know about the merino layout, but that's also likely an issue.
Triage: confirmed by reading the layout records and their consumers. This is a data/metadata bug, not a rendering one, as you suspected.
The invariant. In a tile record, the tiles-array index is the visual slot — CardSections.jsx emits col-N-position-<tileIndex>, and _CardSections.scss turns that into order: <tileIndex>. tile.position is the data index placed in that slot. The spoc's data index is taken only from the columnCount: 1 breakpoint's hasAd tile — selectLayoutRender.mjs. So in every breakpoint the hasAd: true tile must carry the same position as the 1-col hasAd tile.
Root cause. For 4-medium-small-1-ad the 1-col ad is position: 1, but the columnCount: 4 breakpoint flags hasAd: true on tiles[3], which has position: 3 — SectionsLayoutFeed.sys.mjs#533 and newtab-section-layouts.json#204. At 4 columns the spoc therefore actually renders in slot 1 (a tile marked hasAd: false) while the tile flagged as the ad renders an organic story. Invisible because all four 4-col tiles are medium/hasExcerpt: true.
Proposed fix. Per your suggestion, make tiles[3] the ad at position: 1 and shift the rest, i.e. 4-col positions become [0, 2, 3, 1] — that keeps the ad in the last visual slot and makes it the tile the spoc actually lands on. Apply to both the hard-coded record and the RS dump; the authoritative newtab-section-layouts RS collection (and Merino's generator) need the same correction, since the dump is only a snapshot.
Same defect in the other records (1-col baseline → offending breakpoint): 6-small-medium-1-ad @ 3-col flags the ad at position: 3 (baseline 1); 7-double-row-2-ad @ 4-col flags its 2nd ad at position: 4 (baseline 5) and @ 3-col flags its 1st ad at position: 2 (baseline 1). Only 4-large-small-medium-1-ad is consistent. Worth fixing in the same pass, though which slot the ad should occupy there is a design call.
Hardening. isValidLayout only checks that columnCount 1–4 are present; it never inspects tiles. Adding a cross-breakpoint check there (reject or warn when a breakpoint's hasAd positions differ from the 1-col set, also catching duplicate/missing position values) plus a unit test over SectionsLayoutManager.DEFAULT_SECTION_LAYOUT would stop this class of typo. No existing test asserts this invariant — every ad fixture in the tree defines only the columnCount: 1 breakpoint.
Note: reasoned from the code, not built or run.
Suggested severity: S4 — metadata-only, no visible or functional effect.
If you'd like to provide feedback on this comment, please use the 👍 or 👎 reaction.
If you want to categorize your feedback you can add one of the following tags: ai-triage-wrong-file, ai-triage-wrong-cause, ai-triage-hallucination, ai-triage-out-of-scope.
Description
•