Last modified: 2014-06-16 18:35:47 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T68616, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 66616 - Parsoid: CSS selectors too broad, add borders to inline images in captions
Parsoid: CSS selectors too broad, add borders to inline images in captions
Status: RESOLVED FIXED
Product: Parsoid
Classification: Unclassified
General (Other open bugs)
unspecified
All All
: Normal normal
: ---
Assigned To: Gabriel Wicke
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-06-14 14:27 UTC by Ed Sanders
Modified: 2014-06-16 18:35 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Ed Sanders 2014-06-14 14:27:18 UTC
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
Comment 1 Ed Sanders 2014-06-15 11:03:25 UTC
VE workaround here: https://gerrit.wikimedia.org/r/139675
Comment 2 Gabriel Wicke 2014-06-16 15:44:31 UTC
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?
Comment 3 C. Scott Ananian 2014-06-16 16:04:54 UTC
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).
Comment 4 Gabriel Wicke 2014-06-16 16:10:16 UTC
(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.
Comment 5 Ed Sanders 2014-06-16 17:39:26 UTC
"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)
Comment 6 Gerrit Notification Bot 2014-06-16 17:51:51 UTC
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
Comment 7 C. Scott Ananian 2014-06-16 17:53:35 UTC
@Ed: right.  That's why you should use the selector given in comment 3; that is, use child selectors, not ancestor selectors.
Comment 8 Gabriel Wicke 2014-06-16 17:56:25 UTC
(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).
Comment 9 Gerrit Notification Bot 2014-06-16 18:34:12 UTC
Change 139867 merged by jenkins-bot:
Minor: more specific parsoid image styling with child selectors

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

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


Navigation
Links