Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Edge (chromium) support #24379

Merged
merged 3 commits into from Apr 23, 2019
Merged

Add Edge (chromium) support #24379

merged 3 commits into from Apr 23, 2019

Conversation

@810
Copy link
Contributor

@810 810 commented Mar 27, 2019

no test

Pull Request for Issue # .

Summary of Changes

add support new edge

Testing Instructions

code review

Expected result

Actual result

Documentation Changes Required

@mbabker
Copy link
Contributor

@mbabker mbabker commented Mar 28, 2019

For completeness, joomla-framework/application#86 added the same for the Joomla\Application\Web\WebClient class and I went ahead and tagged a new release with that included. So if someone wants to do the composer update then both browser detection libraries will have this addition once this PR and the upstream change are applied.

*/
elseif (preg_match('|Edg\/([0-9.]+)|', $this->agent, $version))
{
$this->setBrowser('edg');
Copy link
Contributor

@Quy Quy Mar 28, 2019

Suggested change
$this->setBrowser('edg');
$this->setBrowser('edg');

@810
Copy link
Contributor Author

@810 810 commented Mar 28, 2019

fixed the codestyle

@810 810 mentioned this pull request Apr 1, 2019
@Quy
Copy link
Contributor

@Quy Quy commented Apr 1, 2019

How about using existing code to handle the new version as follows:

			if (preg_match('|Edg(e)?\/([0-9.]+)|', $this->agent, $version))
			{
				$this->setBrowser('edg' . $version[1]);

				if (strpos($version[2], '.') !== false)
				{
					list($this->majorVersion, $this->minorVersion) = explode('.', $version[2]);
				}
				else
				{
					$this->majorVersion = $version[2];
					$this->minorVersion = 0;
				}
			}

@810
Copy link
Contributor Author

@810 810 commented Apr 1, 2019

if (preg_match('|Edg(e)?\/([0-9.]+)|', $this->agent, $version))

Edge will stay at 2 different versions <75 Will be the edge html, and above 75 will use the blink html.

@Quy
Copy link
Contributor

@Quy Quy commented Apr 1, 2019

$this->setBrowser('edg' . $version[1]);

It will be set to edge or edg accordingly.

@ghost ghost added J3 Issue and removed J3 Issue labels Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@HLeithner HLeithner merged commit aa9daa3 into joomla:staging Apr 23, 2019
3 checks passed
@HLeithner
Copy link
Member

@HLeithner HLeithner commented Apr 23, 2019

thx

@HLeithner HLeithner added this to the Joomla 3.9.6 milestone Apr 23, 2019
tecpromotion added a commit to tecpromotion/joomla-cms that referenced this issue May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants