Make WordPress Core

Opened 10 years ago

Last modified 3 years ago

#28117 new defect (bug)

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

Reported by: dimadin's profile 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 10 years ago.
28117.1.patch (554 bytes) - added by sabernhardt 3 years ago.

Download all attachments as: .zip

Change History (12)

@dimadin
10 years ago

#1 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 4.0

#2 @toscho
10 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
10 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.


10 years ago

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


10 years ago

#6 @DrewAPicture
10 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
8 years ago

  • Focuses administration added

@sabernhardt
3 years ago

#8 @sabernhardt
3 years 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
3 years ago

  • Milestone set to Awaiting Review

#10 @sabernhardt
3 years 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.