WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 2 weeks ago

#28117 new defect (bug)

Admin bar shouldn't use dynamic styles on frontend for logged out visitors

Reported by: dimadin Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.3
Component: Toolbar Keywords: has-patch needs-testing needs-testing-info
Focuses: administration Cc:

Description

Admin bar gets styles depending on current user's browser. Problem is that when you using admin bar on front end for logged out users with full page cache turned on, visitor with IE or mobile might be the first one so everyone else with get that specific style.

See #26221 for similar problem.

Attachments (2)

28117.patch (1.1 KB) - added by dimadin 7 years ago.
28117.1.patch (554 bytes) - added by sabernhardt 6 months ago.

Download all attachments as: .zip

Change History (12)

@dimadin
7 years ago

#1 @SergeyBiryukov
7 years ago

  • Milestone changed from Awaiting Review to 4.0

#2 @toscho
7 years ago

That’s a general problem in WordPress: a header Vary: User-Agent is never set when the output depends on the user agent.

#3 @obenland
7 years ago

  • Keywords has-patch added
  • Version set to 3.3

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


7 years ago

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


7 years ago

#6 @DrewAPicture
7 years ago

  • Milestone changed from 4.0 to Future Release

As this is not a regression, let's take a look at this in 4.1. Punting.

#7 @chriscct7
6 years ago

  • Focuses administration added

#8 @sabernhardt
6 months ago

  • Keywords needs-testing added

28117.1.patch refreshes the conditional statement (Internet Explorer classes were removed in changeset 47771).

Also, I removed the nojq class from the PHP because the script no longer removes it.

#9 @sabernhardt
6 months ago

  • Milestone set to Awaiting Review

#10 @sabernhardt
2 weeks ago

  • Keywords needs-testing-info added

28117.1.patch still applies, but I have not tested it properly with a caching plugin.

A good testing procedure could help, and below is a basic outline (I'm sure I have missed something):

  • Before applying the patch, we would want to set up page caching, then visit on a mobile device and verify the 'mobile' class is used, then visit again on a desktop and see that the class is still there.
  • After applying the patch, purge the cache, then visit on mobile again to save the mobile version, and visit on a desktop to verify that it does not show the mobile version.
Note: See TracTickets for help on using tickets.