Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 65d1126

Browse files
author
Nurhan Turgut
authored
[web] Fixing launching Safari. This should solve the LUCI issue (#16590)
* Fixing launching Safari. This should solve the LUCI issue * more comments and linking the issue
1 parent fe63094 commit 65d1126

File tree

3 files changed

+21
-29
lines changed

3 files changed

+21
-29
lines changed

lib/web_ui/dev/common.dart

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ abstract class PlatformBinding {
5050
String getChromeExecutablePath(io.Directory versionDir);
5151
String getFirefoxExecutablePath(io.Directory versionDir);
5252
String getFirefoxLatestVersionUrl();
53-
String getSafariSystemExecutablePath();
53+
String getMacApplicationLauncher();
5454
String getCommandToRunEdge();
5555
}
5656

@@ -85,7 +85,7 @@ class _WindowsBinding implements PlatformBinding {
8585
'https://download.mozilla.org/?product=firefox-latest&os=win&lang=en-US';
8686

8787
@override
88-
String getSafariSystemExecutablePath() =>
88+
String getMacApplicationLauncher() =>
8989
throw UnsupportedError('Safari is not supported on Windows');
9090

9191
@override
@@ -120,7 +120,7 @@ class _LinuxBinding implements PlatformBinding {
120120
'https://download.mozilla.org/?product=firefox-latest&os=linux64&lang=en-US';
121121

122122
@override
123-
String getSafariSystemExecutablePath() =>
123+
String getMacApplicationLauncher() =>
124124
throw UnsupportedError('Safari is not supported on Linux');
125125

126126
@override
@@ -161,8 +161,7 @@ class _MacBinding implements PlatformBinding {
161161
'https://download.mozilla.org/?product=firefox-latest&os=osx&lang=en-US';
162162

163163
@override
164-
String getSafariSystemExecutablePath() =>
165-
'/Applications/Safari.app/Contents/MacOS/Safari';
164+
String getMacApplicationLauncher() => 'open';
166165

167166
@override
168167
String getCommandToRunEdge() =>

lib/web_ui/dev/safari.dart

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,8 @@
33
// found in the LICENSE file.
44

55
import 'dart:async';
6-
import 'dart:convert';
76
import 'dart:io';
87

9-
import 'environment.dart';
10-
11-
import 'package:path/path.dart' as path;
12-
import 'package:pedantic/pedantic.dart';
13-
14-
import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
15-
168
import 'browser.dart';
179
import 'safari_installation.dart';
1810
import 'common.dart';
@@ -43,21 +35,22 @@ class Safari extends Browser {
4335
infoLog: DevNull(),
4436
);
4537

46-
// Safari will only open files (not general URLs) via the command-line
47-
// API, so we create a dummy file to redirect it to the page we actually
48-
// want it to load.
49-
final Directory redirectDir = Directory(
50-
path.join(environment.webUiDartToolDir.path),
51-
);
52-
final redirect = path.join(redirectDir.path, 'redirect.html');
53-
File(redirect).writeAsStringSync(
54-
'<script>location = ' + jsonEncode(url.toString()) + '</script>');
55-
56-
var process =
57-
await Process.start(installation.executable, [redirect] /* args */);
58-
59-
unawaited(process.exitCode
60-
.then((_) => File(redirect).deleteSync(recursive: true)));
38+
// In the latest versions of MacOs opening Safari browser with a file brings
39+
// a popup which halts the test.
40+
// The following list of arguments needs to be provided to the `open` command
41+
// to open Safari for a given URL. In summary they provide a new instance
42+
// to open, that instance to wait for opening the url until Safari launches,
43+
// provide Safari bundles identifier.
44+
// The details copied from `man open` on MacOS.
45+
// TODO(nurhan): https://github.com/flutter/flutter/issues/50809
46+
var process = await Process.start(installation.executable, [
47+
'-F', // Open a fresh application with no persistant state.
48+
'-W', // Open to wait until the applications it opens.
49+
'-n', // Open a new instance of the application.
50+
'-b', // Specifies the bundle identifier for the application to use.
51+
'com.apple.Safari', // Bundle identifier for Safari.
52+
'${url.toString()}'
53+
]);
6154

6255
return process;
6356
});

lib/web_ui/dev/safari_installation.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ Future<BrowserInstallation> getOrInstallSafari(
7070
infoLog.writeln('Using the system version that is already installed.');
7171
return BrowserInstallation(
7272
version: 'system',
73-
executable: PlatformBinding.instance.getSafariSystemExecutablePath(),
73+
executable: PlatformBinding.instance.getMacApplicationLauncher(),
7474
);
7575
} else {
7676
infoLog.writeln('Unsupported version $requestedVersion.');

0 commit comments

Comments
 (0)