Page MenuHomePhabricator

Parsoid: CSS selectors too broad, add borders to inline images in captions
Closed, ResolvedPublic

Description

As a result we have to use element name selectors in the CSS which are bad in general, e.g. figure[typeof~='mw:Image/Thumb'] img

In this case for two specific reasons:

  1. an inline image in the caption will now get an unwanted border
  2. an image used by VE as shield will get unwanted margins

Version: unspecified
Severity: normal

Details

Reference
bz66616

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:10 AM
bzimport added a project: Parsoid.
bzimport set Reference to bz66616.
Esanders created this task.Jun 14 2014, 2:27 PM

Can you be more specific on why "figure[typeof~='mw:Image/Thumb'] img" is bad?

If it's performance, then we could make it more specific with

figure[typeof~='mw:Image/Thumb'] > a > img,
figure[typeof~='mw:Image/Thumb'] > span > img

Afaik the original reason for modifying Parsoid thumb HTML was that there was no equivalent styling for figure / figcaption. That is being solved now, so perhaps it's a good time to think about removing the rewriting in VE?

Note that performance of CSS selectors is generally sensitive to the right-hand context.

The point of using a consistent structure in the Parsoid DOM for figures/images is so that you could reliably use:

figure[typeof~='mw:Image/Thumb'] > * > img

or even

figure[typeof~='mw:Image/Thumb'] > *:first-child > img:first-child

instead of needing to do

figure[typeof~='mw:Image/Thumb'] img

(which I agree is bad, since it grabs images nested inside captions, as well as any images which are added to the markup by widgets etc, as well as causing the performance implication of a whole leaf-to-root walk up the DOM for every img tag).

(In reply to C. Scott Ananian from comment #3)

Note that performance of CSS selectors is generally sensitive to the
right-hand context.

Right. My impression is that Ed was concerned about something else though.

"1. an inline image in the caption will now get an unwanted border"

An inline image inside a caption shouldn't have a border, but will get one from this selector.

(2. is currently a concert but shields may soon be killed)

Change 139867 had a related patch set uploaded by GWicke:
Minor: more specific parsoid image styling with child selectors

https://gerrit.wikimedia.org/r/139867

@Ed: right. That's why you should use the selector given in comment 3; that is, use child selectors, not ancestor selectors.

(In reply to C. Scott Ananian from comment #7)

@Ed: right. That's why you should use the selector given in comment 3; that
is, use child selectors, not ancestor selectors.

To clarify, VE should *not* use custom selectors wherever possible. The default Parsoid styles should handle this just fine.

See https://gerrit.wikimedia.org/r/139867 for a tweak that addresses your concern 1).

Change 139867 merged by jenkins-bot:
Minor: more specific parsoid image styling with child selectors

https://gerrit.wikimedia.org/r/139867