Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#47369 closed defect (bug) (fixed)

Admin pages have two viewport meta tags on mobile

Reported by: bettyjj's profile BettyJJ Owned by: whyisjake's profile whyisjake
Milestone: 5.5 Priority: low
Severity: normal Version: 3.3
Component: Administration Keywords: dev-feedback has-patch has-dev-note
Focuses: administration, template Cc:

Description

One is add by wp-admin/admin-header.php:

<meta name="viewport" content="width=device-width,initial-scale=1.0">

Another one is added by wp-admin/includes/admin-filters.php and the content is

<meta name="viewport" id="viewport-meta" content="width=device-width, initial-scale=1">

Use Chrome DevTools to emulate any mobile browser and you can see them.

Having two viewport meta tags is not good practice. Besides, there is really no need to use two viewport meta tags here since their contents are virtually identical.

Attachments (3)

47369.diff (2.7 KB) - added by msaggiorato 5 years ago.
47369.2.diff (3.1 KB) - added by donmhico 5 years ago.
47369.3.diff (3.2 KB) - added by audrasjb 5 years ago.
Patch refresh

Download all attachments as: .zip

Change History (22)

#1 @mukesh27
6 years ago

  • Keywords dev-feedback added
  • Version set to 3.3

Hi @BettyJJ, welcome to WordPress Trac! Thanks for the report.

Also replicated same issue in mobile device.

Second viewport added via wp-admin/includes/admin-filters.php file with add_action( 'admin_head', '_ipad_meta' ); add_action code and it was introduce in WordPress 3.3.0

/**
 * @since 3.3.0
 */
function _ipad_meta() {
	if ( wp_is_mobile() ) {
		?>
		<meta name="viewport" id="viewport-meta" content="width=device-width, initial-scale=1">
		<?php
	}
}

@desrosj can you please review above and add your thought or further guidance.

#2 @SergeyBiryukov
6 years ago

  • Component changed from General to Administration
  • Keywords needs-patch added; dev-feedback removed
  • Milestone changed from Awaiting Review to 5.3

_ipad_meta() introduced in [18926], global <meta name="viewport"> introduced in [26134].

Looks like _ipad_meta() can be deprecated now.

#3 @BettyJJ
6 years ago

I'm glad my report is received so quickly. Thank you.
Between the two, I personally like _ipad_meta()'s add_action approach since it allows users to change or remove it if they need. So, whichever you'd keep, please consider using add_action instead of hardcoding it in admin-header.php. Thank you. 😊

Last edited 6 years ago by BettyJJ (previous) (diff)

#4 @ajayghaghretiya1
6 years ago

If _pad_meta() will be deprecated then do we need to remove all the dependency as in [18926]?

Thanks.

#5 @msaggiorato
5 years ago

I'm working on it

@msaggiorato
5 years ago

#6 @msaggiorato
5 years ago

Ok here's what i did:

  • Removed the viewport call from wp-admin/admin-header.php
  • Added a new function called wp_admin_viewport_meta() (similar to what's already used for wp_login)
  • Added a new hook customize_controls_head to use the same function in wp-admin/customize.php, and to be in line with admin_head and login_head.
  • Added a filter to the new function I added, to accomodate the different meta printed in the customizer.

Hopefully this works, let me know your thoughts.

#7 @msaggiorato
5 years ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core by pento. View the logs.


5 years ago

#9 @talldanwp
5 years ago

  • Keywords needs-patch added; has-patch removed

Hi @msaggiorato, thanks for working on this. The ticket was discussed in the core triage session (slack logs available in the link above).

The patch worked well in testing. There were a few separate improvements to the code that were recommended which I'll summarise:

  • Small improvements to the if statement in wp_admin_viewport_meta, it was recommended that this is flipped to if ( empty( $viewport_meta ) ) and an early return used, so that there's less indentation.
  • The id on the viewport tag (originally added in [18926]) doesn't seem to be in use any more and it looks like it can be removed.
  • Documentation needs to be added for the for the wp_admin_viewport_meta() and _customizer_mobile_viewport_meta() functions, as well as the admin_viewport_meta filter.

Thanks!

#10 @davidbaumwald
5 years ago

  • Keywords dev-feedback added

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 years ago

#12 @davidbaumwald
5 years ago

  • Milestone changed from 5.3 to Future Release

This ticket still needs a patch based on the most recent feedback. Moving to Future Release.

Version 0, edited 5 years ago by davidbaumwald (next)

#13 @valentinbora
5 years ago

  • Focuses administration template added
  • Milestone changed from Future Release to 5.5
  • Priority changed from normal to low

@donmhico
5 years ago

#14 @donmhico
5 years ago

  • Keywords has-patch added; needs-patch removed

Thanks for the report @BettyJJ and for the initial patch @msaggiorato. I've uploaded 47369.2.diff with the feedback mentioned by @talldanwp.

@audrasjb
5 years ago

Patch refresh

#15 @audrasjb
5 years ago

  • Keywords needs-dev-note added

Hi,

47369.3.diff refreshes the patch to add correct @since tags.
Also adding needs-dev-note workflow keyword.

Cheers,
Jb

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


4 years ago

#17 @whyisjake
4 years ago

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

In 48412:

Administration: Remove multiple viewport meta tags from mobile pages.

In addition, add the wp_admin_viewport_meta() function, paired to the admin_viewport_meta filter to control attributes of the meta tag.

Fixes #47369.
Props BettyJJ, mukesh27, SergeyBiryukov, ajayghaghretiya1, msaggiorato, talldanwp, davidbaumwald, donmhico, audrasjb.

#18 @SergeyBiryukov
4 years ago

In 48420:

Docs: Correct documentation for customize_controls_head hook and _customizer_mobile_viewport_meta() function.

Follow-up to [48412].

See #47369.

#19 @justinahinon
4 years ago

  • Keywords has-dev-note added; needs-dev-note removed
Note: See TracTickets for help on using tickets.