Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#35367 closed defect (bug) (fixed)

Documented attributes for audio shortcode misleading (also video).

Reported by: gitlost's profile gitlost Owned by: swissspidy's profile swissspidy
Milestone: 4.5 Priority: normal
Severity: normal Version: 3.6
Component: Media Keywords: has-patch has-unit-tests commit
Focuses: docs Cc:

Description

The header doc for the audio shortcode wp_audio_shortcode function lists class and style as attributes, but they aren't really as they're just ignored if given. Similarly for the video shortcode class attribute.

Attachments (1)

35367.diff (6.8 KB) - added by swissspidy 9 years ago.

Download all attachments as: .zip

Change History (9)

@swissspidy
9 years ago

#1 @swissspidy
9 years ago

  • Keywords has-patch has-unit-tests added
  • Milestone changed from Awaiting Review to 4.5
  • Version changed from 3.9 to 3.6

Yeah, that sounds like a bug.

35367.diff does:

  • Make sure class and style attributes get correctly passed
  • Fixes the default values in the DocBlocks
  • Make sure the DocBlock for the class attribute filters are right before the apply_filters call
  • Adds unit tests

#2 follow-up: @gitlost
9 years ago

Wow that was quick! Would the filters best be kept though for BC, eg apply_filters( 'wp_audio_shortcode_class', $atts['class'] )?

#3 in reply to: ↑ 2 @swissspidy
9 years ago

Replying to gitlost:

Wow that was quick! Would the filters best be kept though for BC, eg apply_filters( 'wp_audio_shortcode_class', $atts['class'] )?

Yes. As you can see in the patch, I did not remove the filters. I just moved them two lines up.

#4 @gitlost
9 years ago

Oh apologies missed that.

#5 @SergeyBiryukov
9 years ago

  • Keywords commit added

#6 @swissspidy
9 years ago

  • Owner set to swissspidy
  • Resolution set to fixed
  • Status changed from new to closed

In 36240:

Media: Fix wp_audio_shortcode and wp_video_shortcode attributes handling.

Although documented, the class and style attributes were simply ignored.
Adds unit tests.

Fixes #35367.

#7 @swissspidy
9 years ago

In 36241:

Media: After [36240], remove some unneeded whitespace.

Props ocean90.
See #35367.

#8 @johnbillion
8 years ago

In 41229:

Media: Move the Tests_Media::test_video_shortcode_body() method so it runs before other tests in the class that depend on it.

The following tests were never executed as they have @depends annotations which means they get skipped because the test_video_shortcode_body() test has not run by the time they run. Re-ordering the test methods fixes this.

  • test_wp_video_shortcode_with_empty_params()
  • test_wp_video_shortcode_with_bad_attr()
  • test_wp_video_shortcode_attributes()
  • test_wp_video_shortcode_youtube_remove_feature()
  • test_wp_video_shortcode_youtube_force_ssl()
  • test_wp_video_shortcode_vimeo_force_ssl_remove_query_args()
  • test_wp_video_shortcode_vimeo_adds_loop()
  • test_wp_video_shortcode_vimeo_force_adds_loop_true()

See #35367

Note: See TracTickets for help on using tickets.