Make WordPress Core

Opened 5 years ago

Closed 10 months ago

Last modified 10 months ago

#46132 closed defect (bug) (fixed)

Opera changed its render engine to Chrome, $is_opera not working

Reported by: brasofilo's profile brasofilo Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 6.3 Priority: normal
Severity: normal Version: 2.2
Component: General Keywords: has-patch
Focuses: Cc:

Description

Attachments (1)

46132.diff (1.0 KB) - added by brasofilo 5 years ago.
Patch for $is_opera correction

Download all attachments as: .zip

Change History (10)

#1 @desrosj
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 5.0.2 to 2.2

Thanks for the ticket, @brasofilo!

I did some digging, and this Opera check appears to originally have come from b2 (fun!).

Since Opera now uses Chromium, I'd be curious if $is_opera related conditionals throughout core can be removed in favor of $is_chrome. Differences in the browser's behaviors should be looked into, though.

For reference, here is the official documentation for the Opera's user agent.

@brasofilo are you interested in creating a patch for this?

#2 @brasofilo
5 years ago

Thanks for taking a look, @desrosj

I discovered this when I needed a specific CSS rule for Opera (using the body_class filter). And that's because Chrome has an ongoing bug not present in Opera: we can't drag bookmarklets into the bookmarks bar since Chrome 69.
I don't think it'd be a good idea to convert $is_opera to $is_chrome, this kind of mishap among browsers is bound to happen even if everybody changes their render-engine to Chromium :)

I'm attaching the needed patch, hope the format is correct.
Checking for Opera has to be done before Chrome's so as not to give a false positive.

@brasofilo
5 years ago

Patch for $is_opera correction

This ticket was mentioned in PR #4721 on WordPress/wordpress-develop by @costdev.


11 months ago
#3

  • Keywords has-patch added; needs-patch removed

#4 @costdev
11 months ago

  • Milestone changed from Future Release to 6.3

WordPress Beta Tester uses these globals and received a support ticket about this issue.

Given that comment 2 details one difference that existed between Chrome and Opera, the safest option seems to be adding an extra condition and moving the Opera branch above the Chrome branch.

For sure, we should explore the purpose and differences of the two and act accordingly. As this ticket is 4 years old, we should try to make sure there's accurate detection.

PR 4721 refreshes the patch to use str_contains().


Milestoning for 6.3 as the fix, if accepted, is a straightforward one.

#5 @mukesh27
11 months ago

Thanks @costdev for the PR.

I have tested the changes it working fine in my test. Ping @audrasjb for final review.

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


10 months ago

#7 @swissspidy
10 months ago

Patch looks good at a glance, but this could just as well for 6.4.

#8 @peterwilsoncc
10 months ago

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

In 56224:

General: Update Opera browser sniff follow UA string changes.

Since switching to the Blink engine, Opera browsers contain both the strings Chrome and OPR\. This change relocates the Opera test to take place before the Chrome test to ensure the correct browser is detected.

Props brasofilo, desrosj, costdev, mukesh27, swissspidy.
Fixes #46132.

Note: See TracTickets for help on using tickets.