Differential D13378

Handle when the added node itself is audio/video
ClosedPublic
Actions

Authored by broulik on Jun 6 2018, 9:52 AM.

Details

Reviewers
fvogt
davidedmundson
Group Reviewers
Plasma
Commits
R856:a5c049a1a6c6: Handle when the added node itself is audio/video
Summary

We were only looking at its children but we may also get an audio/video element added directly

Test Plan

5.13 branch
Fixes media controls on SoundCloud for me

Diff Detail

Repository
R856 Plasma Browser Integration
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Jun 6 2018, 9:52 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 6 2018, 9:52 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Jun 6 2018, 9:52 AM
fvogt added a comment.Jun 6 2018, 10:01 AM

I expected that querySelectorAll also looks at the element itself, apparently that's not the case...

fvogt requested changes to this revision.Jun 6 2018, 10:01 AM
fvogt added inline comments.
extension/content-script.js
355

Skip the children check if it's a video/audio element.

356

Use node.matches("video,audio") instead?

This revision now requires changes to proceed.Jun 6 2018, 10:01 AM
broulik updated this revision to Diff 35674.Jun 6 2018, 10:07 AM
  • Simplify
broulik updated this revision to Diff 35675.Jun 6 2018, 10:07 AM
  • Use same selector order as the existing code below
fvogt accepted this revision.Jun 6 2018, 11:21 AM
fvogt added inline comments.
extension/content-script.js
367

I only notice now how stupid this function call actually is...

This revision is now accepted and ready to land.Jun 6 2018, 11:21 AM
Closed by commit R856:a5c049a1a6c6: Handle when the added node itself is audio/video (authored by broulik). · Explain WhyJun 6 2018, 11:33 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mextension/content-script.js (10 lines)

Diff 35674

View Options

extension/content-script.js

Loading...