From 8248db583b016d00eb3a0d50f390d2d8fad57874 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 03:34:03 +0530 Subject: [PATCH 01/11] deps: Update CocoaPods to 1.15.2 --- ios/Podfile.lock | 2 +- macos/Podfile.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ios/Podfile.lock b/ios/Podfile.lock index d9e1a993d0..8a10401389 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -235,4 +235,4 @@ SPEC CHECKSUMS: PODFILE CHECKSUM: 7ed5116924b3be7e8fb75f7aada61e057028f5c7 -COCOAPODS: 1.13.0 +COCOAPODS: 1.15.2 diff --git a/macos/Podfile.lock b/macos/Podfile.lock index a6b02e8755..099f3265a4 100644 --- a/macos/Podfile.lock +++ b/macos/Podfile.lock @@ -174,4 +174,4 @@ SPEC CHECKSUMS: PODFILE CHECKSUM: 353c8bcc5d5b0994e508d035b5431cfe18c1dea7 -COCOAPODS: 1.13.0 +COCOAPODS: 1.15.2 From 4ebdaa0a645e0870ae847c941d3f36dc5c774980 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 03:35:11 +0530 Subject: [PATCH 02/11] content: Handle html parsing of EmbedVideoNode --- lib/model/content.dart | 98 ++++++++++++++++++++++++++++++++++++ lib/widgets/content.dart | 8 +++ test/model/content_test.dart | 93 ++++++++++++++++++++++++++++++++++ 3 files changed, 199 insertions(+) diff --git a/lib/model/content.dart b/lib/model/content.dart index 824d71eac6..992e173a10 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -374,6 +374,50 @@ class ImageNode extends BlockContentNode { } } +class EmbedVideoNode extends BlockContentNode { + const EmbedVideoNode({ + super.debugHtmlNode, + required this.hrefUrl, + required this.previewImageSrcUrl, + }); + + /// A URL string for the video, typically on an external service. + /// + /// For example, this URL may be on youtube.com or vimeo.com. + /// + /// Unlike with [previewImageSrcUrl] or [InlineVideoNode.srcUrl], + /// no requests should be made to this URL unless the user explicitly chooses + /// to interact with the video, in order to protect the user's privacy. + final String hrefUrl; + + /// A URL string for a thumbnail image for the video, on the Zulip server. + /// + /// This may be a relative URL string. It also may not work without adding + /// authentication credentials to the request. + /// + /// Like [InlineVideoNode.srcUrl] and unlike [hrefUrl], this is suitable + /// from a privacy perspective for eagerly fetching data when the user + /// passively scrolls the video into view. + final String previewImageSrcUrl; + + @override + bool operator ==(Object other) { + return other is EmbedVideoNode + && other.hrefUrl == hrefUrl + && other.previewImageSrcUrl == previewImageSrcUrl; + } + + @override + int get hashCode => Object.hash('EmbedVideoNode', hrefUrl, previewImageSrcUrl); + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('hrefUrl', hrefUrl)); + properties.add(StringProperty('previewImageSrcUrl', previewImageSrcUrl)); + } +} + /// A content node that expects an inline layout context from its parent. /// /// When rendered into a Flutter widget tree, an inline content node @@ -968,6 +1012,50 @@ class _ZulipContentParser { return ImageNode(srcUrl: src, debugHtmlNode: debugHtmlNode); } + static final _videoClassNameRegexp = () { + const sourceType = r"(youtube-video|embed-video)"; + return RegExp("^message_inline_image $sourceType|$sourceType message_inline_image\$"); + }(); + + BlockContentNode parseEmbedVideoNode(dom.Element divElement) { + assert(_debugParserContext == _ParserContext.block); + assert(divElement.localName == 'div' + && _videoClassNameRegexp.hasMatch(divElement.className)); + + final pair = () { + if (divElement.nodes.length != 1) return null; + final child = divElement.nodes[0]; + if (child is! dom.Element) return null; + if (child.localName != 'a') return null; + if (child.className.isNotEmpty) return null; + + if (child.nodes.length != 1) return null; + final grandchild = child.nodes[0]; + if (grandchild is! dom.Element) return null; + if (grandchild.localName != 'img') return null; + if (grandchild.className.isNotEmpty) return null; + return (child, grandchild); + }(); + + final debugHtmlNode = kDebugMode ? divElement : null; + if (pair == null) { + return UnimplementedBlockContentNode(htmlNode: divElement); + } + final (anchorElement, imgElement) = pair; + + final imgSrc = imgElement.attributes['src']; + if (imgSrc == null) { + return UnimplementedBlockContentNode(htmlNode: divElement); + } + + final href = anchorElement.attributes['href']; + if (href == null) { + return UnimplementedBlockContentNode(htmlNode: divElement); + } + + return EmbedVideoNode(hrefUrl: href, previewImageSrcUrl: imgSrc, debugHtmlNode: debugHtmlNode); + } + BlockContentNode parseBlockContent(dom.Node node) { assert(_debugParserContext == _ParserContext.block); final debugHtmlNode = kDebugMode ? node : null; @@ -1048,6 +1136,16 @@ class _ZulipContentParser { return parseImageNode(element); } + if (localName == 'div') { + final match = _videoClassNameRegexp.firstMatch(className); + if (match != null) { + final videoClass = match.group(1) ?? match.group(2)!; + if (videoClass case 'youtube-video' || 'embed-video') { + return parseEmbedVideoNode(element); + } + } + } + // TODO more types of node return UnimplementedBlockContentNode(htmlNode: node); } diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index be8cff89a8..c0e3c9332d 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -98,6 +98,14 @@ class BlockContentList extends StatelessWidget { "It should be wrapped in [ImageNodeList]." ); return MessageImage(node: node); + } else if (node is EmbedVideoNode) { + return Text.rich( + TextSpan(children: [ + const TextSpan(text: "(unimplemented:", style: errorStyle), + TextSpan(text: node.debugHtmlText, style: errorCodeStyle), + const TextSpan(text: ")", style: errorStyle), + ]), + style: errorStyle); } else if (node is UnimplementedBlockContentNode) { return Text.rich(_errorUnimplemented(node)); } else { diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 75286f4495..0142e9bd0d 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -613,6 +613,93 @@ class ContentExample { ThematicBreakNode(), ParagraphNode(links: null, nodes: [TextNode('b')]), ]); + + static const videoEmbedYoutube = ContentExample( + 'video preview for youtube embed with thumbnail', + 'https://www.youtube.com/watch?v=aqz-KE-bpKQ', + '

' + 'https://www.youtube.com/watch?v=aqz-KE-bpKQ' + '

\n' + '
' + '' + '
', [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: 'https://www.youtube.com/watch?v=aqz-KE-bpKQ', nodes: [TextNode('https://www.youtube.com/watch?v=aqz-KE-bpKQ')]), + ]), + EmbedVideoNode( + hrefUrl: 'https://www.youtube.com/watch?v=aqz-KE-bpKQ', + previewImageSrcUrl: '/external_content/ecb96e8f884f481c4bc0179287a44ab9014aa78f/68747470733a2f2f692e7974696d672e636f6d2f76692f61717a2d4b452d62704b512f64656661756c742e6a7067' + ), + ]); + + static const videoEmbedYoutubeClassesFlipped = ContentExample( + 'video preview for youtube embed with thumbnail, (hypothetical) class name reorder', + null, // "https://www.youtube.com/watch?v=aqz-KE-bpKQ" (hypothetical server variation) + '

' + 'https://www.youtube.com/watch?v=aqz-KE-bpKQ' + '

\n' + '
' + '' + '
', [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: 'https://www.youtube.com/watch?v=aqz-KE-bpKQ', nodes: [TextNode('https://www.youtube.com/watch?v=aqz-KE-bpKQ')]), + ]), + EmbedVideoNode( + hrefUrl: 'https://www.youtube.com/watch?v=aqz-KE-bpKQ', + previewImageSrcUrl: '/external_content/ecb96e8f884f481c4bc0179287a44ab9014aa78f/68747470733a2f2f692e7974696d672e636f6d2f76692f61717a2d4b452d62704b512f64656661756c742e6a7067' + ), + ]); + + static const videoEmbedVimeoPreviewDisabled = ContentExample( + 'video non-preview for attempted vimeo embed with realm link previews disabled', + 'https://vimeo.com/1084537', + '

' + 'https://vimeo.com/1084537

', [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: 'https://vimeo.com/1084537', nodes: [TextNode('https://vimeo.com/1084537')]), + ]), + ]); + + static const videoEmbedVimeo = ContentExample( + 'video preview for vimeo embed with realm link previews enabled', + 'https://vimeo.com/1084537', + // The server really does generate an attribute called "data-id" whose value + // is a blob of HTML. The web client uses this to show Vimeo's video player + // inside a sandbox iframe. The HTML comes from Vimeo and may change form; + // that's OK the way web uses it, but we shouldn't try to parse it. See: + // https://chat.zulip.org/#narrow/stream/9-issues/topic/Vimeo.20link.20previews.20HTML.20.22data-id.22.20isn't.20a.20.20Vimeo.20video.20ID/near/1767563 + '

' + 'Vimeo - Big Buck Bunny' + '

\n' + '
' + '' + '
', [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: 'https://vimeo.com/1084537', nodes: [TextNode('Vimeo - Big Buck Bunny')]), + ]), + EmbedVideoNode( + hrefUrl: 'https://vimeo.com/1084537', + previewImageSrcUrl: 'https://uploads.zulipusercontent.net/75aed2df4a1e8657176fcd6159fc40876ace4070/68747470733a2f2f692e76696d656f63646e2e636f6d2f766964656f2f32303936333634392d663032383137343536666334386537633331376566346330376261323539636434623430613336343962643865623530613434313862353965633366356166352d645f363430' + ), + ]); + + static const videoEmbedVimeoClassesFlipped = ContentExample( + 'video preview for vimeo embed with realm link previews enabled, (hypothetical) class name reorder', + 'https://vimeo.com/1084537', + '

' + 'Vimeo - Big Buck Bunny' + '

\n' + '
' + '' + '
', [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: 'https://vimeo.com/1084537', nodes: [TextNode('Vimeo - Big Buck Bunny')]), + ]), + EmbedVideoNode( + hrefUrl: 'https://vimeo.com/1084537', + previewImageSrcUrl: 'https://uploads.zulipusercontent.net/75aed2df4a1e8657176fcd6159fc40876ace4070/68747470733a2f2f692e76696d656f63646e2e636f6d2f766964656f2f32303936333634392d663032383137343536666334386537633331376566346330376261323539636434623430613336343962643865623530613434313862353965633366356166352d645f363430' + ), + ]); } UnimplementedBlockContentNode blockUnimplemented(String html) { @@ -925,6 +1012,12 @@ void main() { testParseExample(ContentExample.imageClusterInImplicitParagraph); testParseExample(ContentExample.imageClusterInImplicitParagraphThenContent); + testParseExample(ContentExample.videoEmbedYoutube); + testParseExample(ContentExample.videoEmbedYoutubeClassesFlipped); + testParseExample(ContentExample.videoEmbedVimeoPreviewDisabled); + testParseExample(ContentExample.videoEmbedVimeo); + testParseExample(ContentExample.videoEmbedVimeoClassesFlipped); + testParse('parse nested lists, quotes, headings, code blocks', // "1. > ###### two\n > * three\n\n four" '
    \n
  1. \n
    \n
    two
    \n
      \n
    • three
      • \n' From c1861daa8c750b350b1a9c084ad10530482b48c5 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 03:40:12 +0530 Subject: [PATCH 03/11] content: Handle html parsing of InlineVideoNode --- lib/model/content.dart | 75 ++++++++++++++++++++++++++++++++++-- lib/widgets/content.dart | 8 ++++ test/model/content_test.dart | 32 +++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) diff --git a/lib/model/content.dart b/lib/model/content.dart index 992e173a10..93f057dc1c 100644 --- a/lib/model/content.dart +++ b/lib/model/content.dart @@ -374,6 +374,39 @@ class ImageNode extends BlockContentNode { } } +class InlineVideoNode extends BlockContentNode { + const InlineVideoNode({ + super.debugHtmlNode, + required this.srcUrl, + }); + + /// A URL string for the video resource, on the Zulip server. + /// + /// This may be a relative URL string. It also may not work without adding + /// authentication credentials to the request. + /// + /// Unlike [EmbedVideoNode.hrefUrl], this should always be a URL served by + /// either the Zulip server itself or a service it trusts. It's therefore + /// fine from a privacy perspective to eagerly request data from this resource + /// when the user passively scrolls the video into view. + final String srcUrl; + + @override + bool operator ==(Object other) { + return other is InlineVideoNode + && other.srcUrl == srcUrl; + } + + @override + int get hashCode => Object.hash('InlineVideoNode', srcUrl); + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(StringProperty('srcUrl', srcUrl)); + } +} + class EmbedVideoNode extends BlockContentNode { const EmbedVideoNode({ super.debugHtmlNode, @@ -1013,10 +1046,43 @@ class _ZulipContentParser { } static final _videoClassNameRegexp = () { - const sourceType = r"(youtube-video|embed-video)"; + const sourceType = r"(message_inline_video|youtube-video|embed-video)"; return RegExp("^message_inline_image $sourceType|$sourceType message_inline_image\$"); }(); + BlockContentNode parseInlineVideoNode(dom.Element divElement) { + assert(_debugParserContext == _ParserContext.block); + assert(divElement.localName == 'div' + && _videoClassNameRegexp.hasMatch(divElement.className)); + + final videoElement = () { + if (divElement.nodes.length != 1) return null; + final child = divElement.nodes[0]; + if (child is! dom.Element) return null; + if (child.localName != 'a') return null; + if (child.className.isNotEmpty) return null; + + if (child.nodes.length != 1) return null; + final grandchild = child.nodes[0]; + if (grandchild is! dom.Element) return null; + if (grandchild.localName != 'video') return null; + if (grandchild.className.isNotEmpty) return null; + return grandchild; + }(); + + final debugHtmlNode = kDebugMode ? divElement : null; + if (videoElement == null) { + return UnimplementedBlockContentNode(htmlNode: divElement); + } + + final src = videoElement.attributes['src']; + if (src == null) { + return UnimplementedBlockContentNode(htmlNode: divElement); + } + + return InlineVideoNode(srcUrl: src, debugHtmlNode: debugHtmlNode); + } + BlockContentNode parseEmbedVideoNode(dom.Element divElement) { assert(_debugParserContext == _ParserContext.block); assert(divElement.localName == 'div' @@ -1140,8 +1206,11 @@ class _ZulipContentParser { final match = _videoClassNameRegexp.firstMatch(className); if (match != null) { final videoClass = match.group(1) ?? match.group(2)!; - if (videoClass case 'youtube-video' || 'embed-video') { - return parseEmbedVideoNode(element); + switch (videoClass) { + case 'message_inline_video': + return parseInlineVideoNode(element); + case 'youtube-video' || 'embed-video': + return parseEmbedVideoNode(element); } } } diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index c0e3c9332d..17586faa52 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -98,6 +98,14 @@ class BlockContentList extends StatelessWidget { "It should be wrapped in [ImageNodeList]." ); return MessageImage(node: node); + } else if (node is InlineVideoNode) { + return Text.rich( + TextSpan(children: [ + const TextSpan(text: "(unimplemented:", style: errorStyle), + TextSpan(text: node.debugHtmlText, style: errorCodeStyle), + const TextSpan(text: ")", style: errorStyle), + ]), + style: errorStyle); } else if (node is EmbedVideoNode) { return Text.rich( TextSpan(children: [ diff --git a/test/model/content_test.dart b/test/model/content_test.dart index 0142e9bd0d..5b29c7d552 100644 --- a/test/model/content_test.dart +++ b/test/model/content_test.dart @@ -700,6 +700,36 @@ class ContentExample { previewImageSrcUrl: 'https://uploads.zulipusercontent.net/75aed2df4a1e8657176fcd6159fc40876ace4070/68747470733a2f2f692e76696d656f63646e2e636f6d2f766964656f2f32303936333634392d663032383137343536666334386537633331376566346330376261323539636434623430613336343962643865623530613434313862353965633366356166352d645f363430' ), ]); + + static const videoInline = ContentExample( + 'video preview for user uploaded video', + '[Big-Buck-Bunny.webm](/user_uploads/2/78/_KoRecCHZTFrVtyTKCkIh5Hq/Big-Buck-Bunny.webm)', + '

      ' + 'Big-Buck-Bunny.webm' + '

      \n' + '
      ' + '' + '
      ', [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: '/user_uploads/2/78/_KoRecCHZTFrVtyTKCkIh5Hq/Big-Buck-Bunny.webm', nodes: [TextNode('Big-Buck-Bunny.webm')]), + ]), + InlineVideoNode(srcUrl: '/user_uploads/2/78/_KoRecCHZTFrVtyTKCkIh5Hq/Big-Buck-Bunny.webm'), + ]); + + static const videoInlineClassesFlipped = ContentExample( + 'video preview for user uploaded video, (hypothetical) class name reorder', + '[Big-Buck-Bunny.webm](/user_uploads/2/78/_KoRecCHZTFrVtyTKCkIh5Hq/Big-Buck-Bunny.webm)', + '

      ' + 'Big-Buck-Bunny.webm' + '

      \n' + '
      ' + '' + '
      ', [ + ParagraphNode(links: null, nodes: [ + LinkNode(url: '/user_uploads/2/78/_KoRecCHZTFrVtyTKCkIh5Hq/Big-Buck-Bunny.webm', nodes: [TextNode('Big-Buck-Bunny.webm')]), + ]), + InlineVideoNode(srcUrl: '/user_uploads/2/78/_KoRecCHZTFrVtyTKCkIh5Hq/Big-Buck-Bunny.webm'), + ]); } UnimplementedBlockContentNode blockUnimplemented(String html) { @@ -1017,6 +1047,8 @@ void main() { testParseExample(ContentExample.videoEmbedVimeoPreviewDisabled); testParseExample(ContentExample.videoEmbedVimeo); testParseExample(ContentExample.videoEmbedVimeoClassesFlipped); + testParseExample(ContentExample.videoInline); + testParseExample(ContentExample.videoInlineClassesFlipped); testParse('parse nested lists, quotes, headings, code blocks', // "1. > ###### two\n > * three\n\n four" From 6e8d8b07e82f4d0aeebde3f429f4f62421348a13 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 03:42:34 +0530 Subject: [PATCH 04/11] deps: Add video_player --- ios/Podfile.lock | 7 ++++ macos/Flutter/GeneratedPluginRegistrant.swift | 2 + macos/Podfile.lock | 7 ++++ pubspec.lock | 40 +++++++++++++++++++ pubspec.yaml | 1 + 5 files changed, 57 insertions(+) diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 8a10401389..e16ed49e88 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -139,6 +139,9 @@ PODS: - SwiftyGif (5.4.5) - url_launcher_ios (0.0.1): - Flutter + - video_player_avfoundation (0.0.1): + - Flutter + - FlutterMacOS DEPENDENCIES: - app_settings (from `.symlinks/plugins/app_settings/ios`) @@ -155,6 +158,7 @@ DEPENDENCIES: - share_plus (from `.symlinks/plugins/share_plus/ios`) - sqlite3_flutter_libs (from `.symlinks/plugins/sqlite3_flutter_libs/ios`) - url_launcher_ios (from `.symlinks/plugins/url_launcher_ios/ios`) + - video_player_avfoundation (from `.symlinks/plugins/video_player_avfoundation/darwin`) SPEC REPOS: trunk: @@ -202,6 +206,8 @@ EXTERNAL SOURCES: :path: ".symlinks/plugins/sqlite3_flutter_libs/ios" url_launcher_ios: :path: ".symlinks/plugins/url_launcher_ios/ios" + video_player_avfoundation: + :path: ".symlinks/plugins/video_player_avfoundation/darwin" SPEC CHECKSUMS: app_settings: 017320c6a680cdc94c799949d95b84cb69389ebc @@ -232,6 +238,7 @@ SPEC CHECKSUMS: sqlite3_flutter_libs: 9bfe005308998aeca155330bbc2ea6dddf834a3b SwiftyGif: 706c60cf65fa2bc5ee0313beece843c8eb8194d4 url_launcher_ios: 6116280ddcfe98ab8820085d8d76ae7449447586 + video_player_avfoundation: 02011213dab73ae3687df27ce441fbbcc82b5579 PODFILE CHECKSUM: 7ed5116924b3be7e8fb75f7aada61e057028f5c7 diff --git a/macos/Flutter/GeneratedPluginRegistrant.swift b/macos/Flutter/GeneratedPluginRegistrant.swift index d9af90d6c9..28372ca883 100644 --- a/macos/Flutter/GeneratedPluginRegistrant.swift +++ b/macos/Flutter/GeneratedPluginRegistrant.swift @@ -15,6 +15,7 @@ import path_provider_foundation import share_plus import sqlite3_flutter_libs import url_launcher_macos +import video_player_avfoundation func RegisterGeneratedPlugins(registry: FlutterPluginRegistry) { DeviceInfoPlusMacosPlugin.register(with: registry.registrar(forPlugin: "DeviceInfoPlusMacosPlugin")) @@ -27,4 +28,5 @@ func RegisterGeneratedPlugins(registry: FlutterPluginRegistry) { SharePlusMacosPlugin.register(with: registry.registrar(forPlugin: "SharePlusMacosPlugin")) Sqlite3FlutterLibsPlugin.register(with: registry.registrar(forPlugin: "Sqlite3FlutterLibsPlugin")) UrlLauncherPlugin.register(with: registry.registrar(forPlugin: "UrlLauncherPlugin")) + FVPVideoPlayerPlugin.register(with: registry.registrar(forPlugin: "FVPVideoPlayerPlugin")) } diff --git a/macos/Podfile.lock b/macos/Podfile.lock index 099f3265a4..82f4a0a939 100644 --- a/macos/Podfile.lock +++ b/macos/Podfile.lock @@ -98,6 +98,9 @@ PODS: - sqlite3/rtree - url_launcher_macos (0.0.1): - FlutterMacOS + - video_player_avfoundation (0.0.1): + - Flutter + - FlutterMacOS DEPENDENCIES: - device_info_plus (from `Flutter/ephemeral/.symlinks/plugins/device_info_plus/macos`) @@ -111,6 +114,7 @@ DEPENDENCIES: - share_plus (from `Flutter/ephemeral/.symlinks/plugins/share_plus/macos`) - sqlite3_flutter_libs (from `Flutter/ephemeral/.symlinks/plugins/sqlite3_flutter_libs/macos`) - url_launcher_macos (from `Flutter/ephemeral/.symlinks/plugins/url_launcher_macos/macos`) + - video_player_avfoundation (from `Flutter/ephemeral/.symlinks/plugins/video_player_avfoundation/darwin`) SPEC REPOS: trunk: @@ -148,6 +152,8 @@ EXTERNAL SOURCES: :path: Flutter/ephemeral/.symlinks/plugins/sqlite3_flutter_libs/macos url_launcher_macos: :path: Flutter/ephemeral/.symlinks/plugins/url_launcher_macos/macos + video_player_avfoundation: + :path: Flutter/ephemeral/.symlinks/plugins/video_player_avfoundation/darwin SPEC CHECKSUMS: device_info_plus: ce1b7762849d3ec103d0e0517299f2db7ad60720 @@ -171,6 +177,7 @@ SPEC CHECKSUMS: sqlite3: 02d1f07eaaa01f80a1c16b4b31dfcbb3345ee01a sqlite3_flutter_libs: 8d204ef443cf0d5c1c8b058044eab53f3943a9c5 url_launcher_macos: d2691c7dd33ed713bf3544850a623080ec693d95 + video_player_avfoundation: 02011213dab73ae3687df27ce441fbbcc82b5579 PODFILE CHECKSUM: 353c8bcc5d5b0994e508d035b5431cfe18c1dea7 diff --git a/pubspec.lock b/pubspec.lock index 10a36fe93d..80cbab5b48 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -1201,6 +1201,46 @@ packages: url: "https://pub.dev" source: hosted version: "2.1.4" + video_player: + dependency: "direct main" + description: + name: video_player + sha256: "822e68b62403bebea846b012988ee1e77262e7b786345594a444c59338096ddf" + url: "https://pub.dev" + source: hosted + version: "2.8.4" + video_player_android: + dependency: transitive + description: + name: video_player_android + sha256: "4dd9b8b86d70d65eecf3dcabfcdfbb9c9115d244d022654aba49a00336d540c2" + url: "https://pub.dev" + source: hosted + version: "2.4.12" + video_player_avfoundation: + dependency: transitive + description: + name: video_player_avfoundation + sha256: "309e3962795e761be010869bae65c0b0e45b5230c5cee1bec72197ca7db040ed" + url: "https://pub.dev" + source: hosted + version: "2.5.6" + video_player_platform_interface: + dependency: transitive + description: + name: video_player_platform_interface + sha256: "236454725fafcacf98f0f39af0d7c7ab2ce84762e3b63f2cbb3ef9a7e0550bc6" + url: "https://pub.dev" + source: hosted + version: "6.2.2" + video_player_web: + dependency: transitive + description: + name: video_player_web + sha256: "8e9cb7fe94e49490e67bbc15149691792b58a0ade31b32e3f3688d104a0e057b" + url: "https://pub.dev" + source: hosted + version: "2.2.0" vm_service: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index a70dcde5c4..b593b03784 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -65,6 +65,7 @@ dependencies: sqlite3_flutter_libs: ^0.5.13 url_launcher: ^6.1.11 url_launcher_android: ">=6.1.0" + video_player: ^2.8.3 zulip_plugin: path: ./packages/zulip_plugin # Keep list sorted when adding dependencies; it helps prevent merge conflicts. From 0dbd24a9c098874e661f7a5332cf59a827ed44f4 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 16:06:02 +0530 Subject: [PATCH 05/11] deps: Add dev:plugin_platform_interface directly Used in tests for mocking video_player platform interface --- pubspec.lock | 2 +- pubspec.yaml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pubspec.lock b/pubspec.lock index 80cbab5b48..c7cd1eeb27 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -845,7 +845,7 @@ packages: source: hosted version: "3.1.4" plugin_platform_interface: - dependency: transitive + dependency: "direct dev" description: name: plugin_platform_interface sha256: "4820fbfdb9478b1ebae27888254d445073732dae3d6ea81f0b7e06d5dedc3f02" diff --git a/pubspec.yaml b/pubspec.yaml index b593b03784..c33da60ff3 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -86,6 +86,7 @@ dev_dependencies: flutter_lints: ^3.0.0 json_serializable: ^6.5.4 pigeon: ^18.0.0 + plugin_platform_interface: ^2.1.8 stack_trace: ^1.11.1 test: ^1.23.1 # Keep list sorted when adding dependencies; it helps prevent merge conflicts. From b63db38afb933cfb672965a7e96126b8064526f6 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 16:06:31 +0530 Subject: [PATCH 06/11] deps: Add dev:video_player_platform_interface directly Used in tests for mocking video_player platform interface --- pubspec.lock | 2 +- pubspec.yaml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pubspec.lock b/pubspec.lock index c7cd1eeb27..7550f0eb92 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -1226,7 +1226,7 @@ packages: source: hosted version: "2.5.6" video_player_platform_interface: - dependency: transitive + dependency: "direct dev" description: name: video_player_platform_interface sha256: "236454725fafcacf98f0f39af0d7c7ab2ce84762e3b63f2cbb3ef9a7e0550bc6" diff --git a/pubspec.yaml b/pubspec.yaml index c33da60ff3..1e5183e767 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -89,6 +89,7 @@ dev_dependencies: plugin_platform_interface: ^2.1.8 stack_trace: ^1.11.1 test: ^1.23.1 + video_player_platform_interface: ^6.2.2 # Keep list sorted when adding dependencies; it helps prevent merge conflicts. # For information on the generic Dart part of this file, see the From 4b668ff9ca238abf7dfba3ea68837a9f8d0b6ebb Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 16:09:34 +0530 Subject: [PATCH 07/11] content [nfc]: Pull out MessageMediaContainer widget --- lib/widgets/content.dart | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 17586faa52..53441912b8 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -390,11 +390,34 @@ class MessageImage extends StatelessWidget { final resolvedSrc = store.tryResolveUrl(src); // TODO if src fails to parse, show an explicit "broken image" - return GestureDetector( + return MessageMediaContainer( onTap: resolvedSrc == null ? null : () { // TODO(log) Navigator.of(context).push(getLightboxRoute( context: context, message: message, src: resolvedSrc)); }, + child: resolvedSrc == null ? null : LightboxHero( + message: message, + src: resolvedSrc, + child: RealmContentNetworkImage( + resolvedSrc, + filterQuality: FilterQuality.medium))); + } +} + +class MessageMediaContainer extends StatelessWidget { + const MessageMediaContainer({ + super.key, + required this.onTap, + required this.child, + }); + + final void Function()? onTap; + final Widget? child; + + @override + Widget build(BuildContext context) { + return GestureDetector( + onTap: onTap, child: UnconstrainedBox( alignment: Alignment.centerLeft, child: Padding( @@ -408,12 +431,7 @@ class MessageImage extends StatelessWidget { child: SizedBox( height: 100, width: 150, - child: resolvedSrc == null ? null : LightboxHero( - message: message, - src: resolvedSrc, - child: RealmContentNetworkImage( - resolvedSrc, - filterQuality: FilterQuality.medium)))))))); + child: child)))))); } } From 1b7ae7f505e3eafdd4af4429157d804526a0e15e Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 5 Apr 2024 16:21:28 +0530 Subject: [PATCH 08/11] content: Implement embed video preview Partially implements #356, provides video thumbnail previews for embedded external videos (Youtube & Vimeo). --- lib/widgets/content.dart | 37 +++++++++++++++++++++----- test/widgets/content_test.dart | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 53441912b8..89da2d4baf 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -107,13 +107,7 @@ class BlockContentList extends StatelessWidget { ]), style: errorStyle); } else if (node is EmbedVideoNode) { - return Text.rich( - TextSpan(children: [ - const TextSpan(text: "(unimplemented:", style: errorStyle), - TextSpan(text: node.debugHtmlText, style: errorCodeStyle), - const TextSpan(text: ")", style: errorStyle), - ]), - style: errorStyle); + return MessageEmbedVideo(node: node); } else if (node is UnimplementedBlockContentNode) { return Text.rich(_errorUnimplemented(node)); } else { @@ -404,6 +398,35 @@ class MessageImage extends StatelessWidget { } } +class MessageEmbedVideo extends StatelessWidget { + const MessageEmbedVideo({super.key, required this.node}); + + final EmbedVideoNode node; + + @override + Widget build(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final previewImageSrcUrl = store.tryResolveUrl(node.previewImageSrcUrl); + + return MessageMediaContainer( + onTap: () => _launchUrl(context, node.hrefUrl), + child: Stack( + alignment: Alignment.center, + children: [ + if (previewImageSrcUrl != null) // TODO(log) + RealmContentNetworkImage( + previewImageSrcUrl, + filterQuality: FilterQuality.medium), + // Show the "play" icon even when previewImageSrcUrl didn't resolve; + // the action uses hrefUrl, which might still work. + const Icon( + Icons.play_arrow_rounded, + color: Colors.white, + size: 32), + ])); + } +} + class MessageMediaContainer extends StatelessWidget { const MessageMediaContainer({ super.key, diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 4277416d73..6b26b6e8e7 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -328,6 +328,54 @@ void main() { }); }); + group("MessageEmbedVideo", () { + Future prepareContent(WidgetTester tester, String html) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + prepareBoringImageHttpClient(); + + await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp( + home: PerAccountStoreWidget(accountId: eg.selfAccount.id, + child: MessageContent( + message: eg.streamMessage(content: html), + content: parseContent(html)))))); + await tester.pump(); // global store + await tester.pump(); // per-account store + debugNetworkImageHttpClientProvider = null; + } + + Future checkEmbedVideo(WidgetTester tester, ContentExample example) async { + await prepareContent(tester, example.html); + + final expectedTitle = (((example.expectedNodes[0] as ParagraphNode) + .nodes.single as LinkNode).nodes.single as TextNode).text; + await tester.ensureVisible(find.text(expectedTitle)); + + final expectedVideo = example.expectedNodes[1] as EmbedVideoNode; + final expectedResolvedUrl = eg.store() + .tryResolveUrl(expectedVideo.previewImageSrcUrl)!; + final image = tester.widget( + find.byType(RealmContentNetworkImage)); + check(image.src).equals(expectedResolvedUrl); + + final expectedLaunchUrl = expectedVideo.hrefUrl; + await tester.tap(find.byIcon(Icons.play_arrow_rounded)); + check(testBinding.takeLaunchUrlCalls()) + .single.equals((url: Uri.parse(expectedLaunchUrl), mode: LaunchMode.platformDefault)); + } + + testWidgets('video preview for youtube embed', (tester) async { + const example = ContentExample.videoEmbedYoutube; + await checkEmbedVideo(tester, example); + }); + + testWidgets('video preview for vimeo embed', (tester) async { + const example = ContentExample.videoEmbedVimeo; + await checkEmbedVideo(tester, example); + }); + }); + + group("CodeBlock", () { testContentSmoke(ContentExample.codeBlockPlain); testContentSmoke(ContentExample.codeBlockHighlightedShort); From 9806c5ad817ef99ba3a1307b605685966c72c870 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Sun, 5 May 2024 13:43:11 +0530 Subject: [PATCH 09/11] lightbox [nfc]: Pull out _LightboxPageLayout --- lib/widgets/lightbox.dart | 76 +++++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 23 deletions(-) diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index ca133475ce..c85de36523 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -83,22 +83,25 @@ class _CopyLinkButton extends StatelessWidget { } } -class _LightboxPage extends StatefulWidget { - const _LightboxPage({ +class _LightboxPageLayout extends StatefulWidget { + const _LightboxPageLayout({ required this.routeEntranceAnimation, required this.message, - required this.src, + required this.buildBottomAppBar, + required this.child, }); final Animation routeEntranceAnimation; final Message message; - final Uri src; + final Widget? Function( + BuildContext context, Color appBarBackgroundColor, double appBarElevation) buildBottomAppBar; + final Widget child; @override - State<_LightboxPage> createState() => _LightboxPageState(); + State<_LightboxPageLayout> createState() => _LightboxPageLayoutState(); } -class _LightboxPageState extends State<_LightboxPage> { +class _LightboxPageLayoutState extends State<_LightboxPageLayout> { // TODO(#38): Animate entrance/exit of header and footer bool _headerFooterVisible = false; @@ -168,14 +171,8 @@ class _LightboxPageState extends State<_LightboxPage> { Widget? bottomAppBar; if (_headerFooterVisible) { - bottomAppBar = BottomAppBar( - color: appBarBackgroundColor, - elevation: appBarElevation, - child: Row(children: [ - _CopyLinkButton(url: widget.src), - // TODO(#43): Share image - // TODO(#42): Download image - ])); + bottomAppBar = widget.buildBottomAppBar( + context, appBarBackgroundColor, appBarElevation); } return Theme( @@ -186,6 +183,7 @@ class _LightboxPageState extends State<_LightboxPage> { extendBody: true, // For the BottomAppBar extendBodyBehindAppBar: true, // For the AppBar appBar: appBar, + bottomNavigationBar: bottomAppBar, body: MediaQuery( // Clobber the MediaQueryData prepared by Scaffold with one that's not // affected by the app bars. On this screen, the app bars are @@ -197,14 +195,46 @@ class _LightboxPageState extends State<_LightboxPage> { child: GestureDetector( behavior: HitTestBehavior.translucent, onTap: _handleTap, - child: SizedBox.expand( - child: InteractiveViewer( - child: SafeArea( - child: LightboxHero( - message: widget.message, - src: widget.src, - child: RealmContentNetworkImage(widget.src, filterQuality: FilterQuality.medium))))))), - bottomNavigationBar: bottomAppBar)); + child: widget.child)))); + } +} + +class _ImageLightboxPage extends StatefulWidget { + const _ImageLightboxPage({ + required this.routeEntranceAnimation, + required this.message, + required this.src, + }); + + final Animation routeEntranceAnimation; + final Message message; + final Uri src; + + @override + State<_ImageLightboxPage> createState() => _ImageLightboxPageState(); +} + +class _ImageLightboxPageState extends State<_ImageLightboxPage> { + @override + Widget build(BuildContext context) { + return _LightboxPageLayout( + routeEntranceAnimation: widget.routeEntranceAnimation, + message: widget.message, + buildBottomAppBar: (context, color, elevation) => BottomAppBar( + color: color, + elevation: elevation, + child: Row(children: [ + _CopyLinkButton(url: widget.src), + // TODO(#43): Share image + // TODO(#42): Download image + ])), + child: SizedBox.expand( + child: InteractiveViewer( + child: SafeArea( + child: LightboxHero( + message: widget.message, + src: widget.src, + child: RealmContentNetworkImage(widget.src, filterQuality: FilterQuality.medium)))))); } } @@ -224,7 +254,7 @@ Route getLightboxRoute({ Animation secondaryAnimation, ) { // TODO(#40): Drag down to close? - return _LightboxPage(routeEntranceAnimation: animation, message: message, src: src); + return _ImageLightboxPage(routeEntranceAnimation: animation, message: message, src: src); }, transitionsBuilder: ( BuildContext context, From d5ece54fe367e53205402ec1917fe916f4b8c8eb Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Sun, 5 May 2024 13:43:47 +0530 Subject: [PATCH 10/11] content: Implement inline video preview Fixes: #356 --- assets/l10n/app_en.arb | 4 + lib/widgets/content.dart | 44 +++++- lib/widgets/dialog.dart | 5 +- lib/widgets/lightbox.dart | 271 ++++++++++++++++++++++++++++++-- test/widgets/content_test.dart | 34 +++- test/widgets/lightbox_test.dart | 156 ++++++++++++++++++ 6 files changed, 494 insertions(+), 20 deletions(-) create mode 100644 test/widgets/lightbox_test.dart diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index f2686f9bd9..46a356b2d8 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -381,6 +381,10 @@ "httpStatus": {"type": "int", "example": "500"} } }, + "errorVideoPlayerFailed": "Unable to play the video", + "@errorVideoPlayerFailed": { + "description": "Error message when a video fails to play." + }, "serverUrlValidationErrorEmpty": "Please enter a URL.", "@serverUrlValidationErrorEmpty": { "description": "Error message when URL is empty" diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 89da2d4baf..d153860ac1 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -99,13 +99,7 @@ class BlockContentList extends StatelessWidget { ); return MessageImage(node: node); } else if (node is InlineVideoNode) { - return Text.rich( - TextSpan(children: [ - const TextSpan(text: "(unimplemented:", style: errorStyle), - TextSpan(text: node.debugHtmlText, style: errorCodeStyle), - const TextSpan(text: ")", style: errorStyle), - ]), - style: errorStyle); + return MessageInlineVideo(node: node); } else if (node is EmbedVideoNode) { return MessageEmbedVideo(node: node); } else if (node is UnimplementedBlockContentNode) { @@ -387,7 +381,10 @@ class MessageImage extends StatelessWidget { return MessageMediaContainer( onTap: resolvedSrc == null ? null : () { // TODO(log) Navigator.of(context).push(getLightboxRoute( - context: context, message: message, src: resolvedSrc)); + context: context, + message: message, + src: resolvedSrc, + mediaType: MediaType.image)); }, child: resolvedSrc == null ? null : LightboxHero( message: message, @@ -398,6 +395,37 @@ class MessageImage extends StatelessWidget { } } +class MessageInlineVideo extends StatelessWidget { + const MessageInlineVideo({super.key, required this.node}); + + final InlineVideoNode node; + + @override + Widget build(BuildContext context) { + final message = InheritedMessage.of(context); + final store = PerAccountStoreWidget.of(context); + final resolvedSrc = store.tryResolveUrl(node.srcUrl); + + return MessageMediaContainer( + onTap: resolvedSrc == null ? null : () { // TODO(log) + Navigator.of(context).push(getLightboxRoute( + context: context, + message: message, + src: resolvedSrc, + mediaType: MediaType.video)); + }, + child: Container( + color: Colors.black, + alignment: Alignment.center, + // To avoid potentially confusing UX, do not show play icon as + // we also disable onTap above. + child: resolvedSrc == null ? null : const Icon( // TODO(log) + Icons.play_arrow_rounded, + color: Colors.white, + size: 32))); + } +} + class MessageEmbedVideo extends StatelessWidget { const MessageEmbedVideo({super.key, required this.node}); diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index be2fca21f5..edab8ebdbf 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -19,16 +19,19 @@ Future showErrorDialog({ required BuildContext context, required String title, String? message, + bool barrierDismissible = true, + VoidCallback? onContinue, }) { final zulipLocalizations = ZulipLocalizations.of(context); return showDialog( context: context, + barrierDismissible: barrierDismissible, builder: (BuildContext context) => AlertDialog( title: Text(title), content: message != null ? SingleChildScrollView(child: Text(message)) : null, actions: [ TextButton( - onPressed: () => Navigator.pop(context), + onPressed: onContinue ?? () => Navigator.pop(context), child: _dialogActionText(zulipLocalizations.errorDialogContinue)), ])); } diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index c85de36523..818437cb28 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -2,9 +2,13 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:intl/intl.dart'; +import 'package:video_player/video_player.dart'; +import '../api/core.dart'; import '../api/model/model.dart'; +import '../log.dart'; import 'content.dart'; +import 'dialog.dart'; import 'page.dart'; import 'clipboard.dart'; import 'store.dart'; @@ -94,7 +98,7 @@ class _LightboxPageLayout extends StatefulWidget { final Animation routeEntranceAnimation; final Message message; final Widget? Function( - BuildContext context, Color appBarBackgroundColor, double appBarElevation) buildBottomAppBar; + BuildContext context, Color color, double elevation) buildBottomAppBar; final Widget child; @override @@ -108,6 +112,7 @@ class _LightboxPageLayoutState extends State<_LightboxPageLayout> { @override void initState() { super.initState(); + _handleRouteEntranceAnimationStatusChange(widget.routeEntranceAnimation.status); widget.routeEntranceAnimation.addStatusListener(_handleRouteEntranceAnimationStatusChange); } @@ -215,19 +220,24 @@ class _ImageLightboxPage extends StatefulWidget { } class _ImageLightboxPageState extends State<_ImageLightboxPage> { + Widget _buildBottomAppBar(BuildContext context, Color color, double elevation) { + return BottomAppBar( + color: color, + elevation: elevation, + child: Row(children: [ + _CopyLinkButton(url: widget.src), + // TODO(#43): Share image + // TODO(#42): Download image + ]), + ); + } + @override Widget build(BuildContext context) { return _LightboxPageLayout( routeEntranceAnimation: widget.routeEntranceAnimation, message: widget.message, - buildBottomAppBar: (context, color, elevation) => BottomAppBar( - color: color, - elevation: elevation, - child: Row(children: [ - _CopyLinkButton(url: widget.src), - // TODO(#43): Share image - // TODO(#42): Download image - ])), + buildBottomAppBar: _buildBottomAppBar, child: SizedBox.expand( child: InteractiveViewer( child: SafeArea( @@ -238,11 +248,243 @@ class _ImageLightboxPageState extends State<_ImageLightboxPage> { } } +class _VideoPositionSliderControl extends StatefulWidget { + final VideoPlayerController controller; + + const _VideoPositionSliderControl({ + required this.controller, + }); + + @override + State<_VideoPositionSliderControl> createState() => _VideoPositionSliderControlState(); +} + +class _VideoPositionSliderControlState extends State<_VideoPositionSliderControl> { + Duration _sliderValue = Duration.zero; + bool _isSliderDragging = false; + + @override + void initState() { + super.initState(); + _sliderValue = widget.controller.value.position; + widget.controller.addListener(_handleVideoControllerUpdate); + } + + @override + void dispose() { + widget.controller.removeListener(_handleVideoControllerUpdate); + super.dispose(); + } + + void _handleVideoControllerUpdate() { + setState(() { + // After 'controller.seekTo' is called in 'Slider.onChangeEnd' the + // position indicator switches back to the actual controller's position + // but since the call 'seekTo' completes before the actual controller + // updates are notified, the position indicator that switches to controller's + // position can show the older position before the call to 'seekTo' for a + // single frame, resulting in a glichty UX. + // + // To avoid that, we delay the position indicator switch from '_sliderValue' to + // happen when we are notified of the controller update. + if (_isSliderDragging && _sliderValue == widget.controller.value.position) { + _isSliderDragging = false; + } + }); + } + + static String _formatDuration(Duration value) { + final hours = value.inHours.toString().padLeft(2, '0'); + final minutes = value.inMinutes.remainder(60).toString().padLeft(2, '0'); + final seconds = value.inSeconds.remainder(60).toString().padLeft(2, '0'); + return '${hours == '00' ? '' : '$hours:'}$minutes:$seconds'; + } + + @override + Widget build(BuildContext context) { + final currentPosition = _isSliderDragging + ? _sliderValue + : widget.controller.value.position; + + return Row(children: [ + Text(_formatDuration(currentPosition), + style: const TextStyle(color: Colors.white)), + Expanded( + child: Slider( + value: currentPosition.inMilliseconds.toDouble(), + max: widget.controller.value.duration.inMilliseconds.toDouble(), + activeColor: Colors.white, + onChangeStart: (value) { + setState(() { + _sliderValue = Duration(milliseconds: value.toInt()); + _isSliderDragging = true; + }); + }, + onChanged: (value) { + setState(() { + _sliderValue = Duration(milliseconds: value.toInt()); + }); + }, + onChangeEnd: (value) { + final durationValue = Duration(milliseconds: value.toInt()); + setState(() { + _sliderValue = durationValue; + }); + widget.controller.seekTo(durationValue); + + // The toggling back of '_isSliderDragging' is omitted here intentionally, + // see '_handleVideoControllerUpdates'. + }, + ), + ), + Text(_formatDuration(widget.controller.value.duration), + style: const TextStyle(color: Colors.white)), + ]); + } +} + +class VideoLightboxPage extends StatefulWidget { + const VideoLightboxPage({ + super.key, + required this.routeEntranceAnimation, + required this.message, + required this.src, + }); + + final Animation routeEntranceAnimation; + final Message message; + final Uri src; + + @override + State createState() => _VideoLightboxPageState(); +} + +class _VideoLightboxPageState extends State with PerAccountStoreAwareStateMixin { + VideoPlayerController? _controller; + + @override + void onNewStore() { + if (_controller != null) { + // The exclusion of reinitialization logic is deliberate here, + // as initialization relies only on the initial values of the store's + // realm URL and the user's credentials, which we assume remain unchanged + // when the store is replaced. + return; + } + + _initialize(); + } + + Future _initialize() async { + final store = PerAccountStoreWidget.of(context); + + assert(debugLog('VideoPlayerController.networkUrl(${widget.src})')); + _controller = VideoPlayerController.networkUrl(widget.src, httpHeaders: { + if (widget.src.origin == store.account.realmUrl.origin) ...authHeader( + email: store.account.email, + apiKey: store.account.apiKey, + ), + ...userAgentHeader() + }); + _controller!.addListener(_handleVideoControllerUpdate); + + try { + await _controller!.initialize(); + if (_controller == null) return; // widget was disposed + await _controller!.play(); + } catch (error) { // TODO(log) + assert(debugLog("VideoPlayerController.initialize failed: $error")); + if (mounted) { + final zulipLocalizations = ZulipLocalizations.of(context); + await showErrorDialog( + context: context, + title: zulipLocalizations.errorDialogTitle, + message: zulipLocalizations.errorVideoPlayerFailed, + // To avoid showing the disabled video lightbox for the unnsupported + // video, we make sure user doesn't reach there by dismissing the dialog + // by clicking around it, user must press the 'OK' button, which will + // take user back to content message list. + barrierDismissible: false, + onContinue: () { + Navigator.pop(context); // Pops the dialog + Navigator.pop(context); // Pops the lightbox + }); + } + } + } + + @override + void dispose() { + _controller?.removeListener(_handleVideoControllerUpdate); + _controller?.dispose(); + _controller = null; + super.dispose(); + } + + void _handleVideoControllerUpdate() { + setState(() {}); + } + + Widget? _buildBottomAppBar(BuildContext context, Color color, double elevation) { + if (_controller == null) return null; + return BottomAppBar( + height: 150, + color: color, + elevation: elevation, + child: Column(mainAxisAlignment: MainAxisAlignment.end, children: [ + _VideoPositionSliderControl(controller: _controller!), + IconButton( + onPressed: () { + if (_controller!.value.isPlaying) { + _controller!.pause(); + } else { + _controller!.play(); + } + }, + icon: Icon( + _controller!.value.isPlaying + ? Icons.pause_circle_rounded + : Icons.play_circle_rounded, + size: 50, + ), + ), + ]), + ); + } + + @override + Widget build(BuildContext context) { + return _LightboxPageLayout( + routeEntranceAnimation: widget.routeEntranceAnimation, + message: widget.message, + buildBottomAppBar: _buildBottomAppBar, + child: SafeArea( + child: Center( + child: Stack(alignment: Alignment.center, children: [ + if (_controller != null && _controller!.value.isInitialized) + AspectRatio( + aspectRatio: _controller!.value.aspectRatio, + child: VideoPlayer(_controller!)), + if (_controller == null || !_controller!.value.isInitialized || _controller!.value.isBuffering) + const SizedBox( + width: 32, + height: 32, + child: CircularProgressIndicator(color: Colors.white)), + ])))); + } +} + +enum MediaType { + video, + image +} + Route getLightboxRoute({ int? accountId, BuildContext? context, required Message message, required Uri src, + required MediaType mediaType, }) { return AccountPageRouteBuilder( accountId: accountId, @@ -254,7 +496,16 @@ Route getLightboxRoute({ Animation secondaryAnimation, ) { // TODO(#40): Drag down to close? - return _ImageLightboxPage(routeEntranceAnimation: animation, message: message, src: src); + return switch (mediaType) { + MediaType.image => _ImageLightboxPage( + routeEntranceAnimation: animation, + message: message, + src: src), + MediaType.video => VideoLightboxPage( + routeEntranceAnimation: animation, + message: message, + src: src), + }; }, transitionsBuilder: ( BuildContext context, diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 6b26b6e8e7..63a75c677c 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -328,6 +328,39 @@ void main() { }); }); + group("MessageInlineVideo", () { + Future>> prepareContent(WidgetTester tester, String html) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + + final pushedRoutes = >[]; + final testNavObserver = TestNavigatorObserver() + ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + + await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp( + navigatorObservers: [testNavObserver], + home: PerAccountStoreWidget(accountId: eg.selfAccount.id, + child: MessageContent( + message: eg.streamMessage(content: html), + content: parseContent(html)))))); + await tester.pump(); // global store + await tester.pump(); // per-account store + + assert(pushedRoutes.length == 1); + pushedRoutes.removeLast(); + return pushedRoutes; + } + + testWidgets('tapping on preview opens lightbox', (tester) async { + const example = ContentExample.videoInline; + final pushedRoutes = await prepareContent(tester, example.html); + + await tester.tap(find.byIcon(Icons.play_arrow_rounded)); + check(pushedRoutes).single.isA() + .fullscreenDialog.isTrue(); // opened lightbox + }); + }); + group("MessageEmbedVideo", () { Future prepareContent(WidgetTester tester, String html) async { addTearDown(testBinding.reset); @@ -375,7 +408,6 @@ void main() { }); }); - group("CodeBlock", () { testContentSmoke(ContentExample.codeBlockPlain); testContentSmoke(ContentExample.codeBlockHighlightedShort); diff --git a/test/widgets/lightbox_test.dart b/test/widgets/lightbox_test.dart new file mode 100644 index 0000000000..e314c2fd71 --- /dev/null +++ b/test/widgets/lightbox_test.dart @@ -0,0 +1,156 @@ +import 'dart:async'; + +import 'package:checks/checks.dart'; +import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter/material.dart'; +import 'package:plugin_platform_interface/plugin_platform_interface.dart'; +import 'package:video_player_platform_interface/video_player_platform_interface.dart'; +import 'package:video_player/video_player.dart'; +import 'package:zulip/widgets/lightbox.dart'; +import 'package:zulip/widgets/store.dart'; + +import '../example_data.dart' as eg; +import '../model/binding.dart'; + +class FakeVideoPlayerPlatform extends Fake + with MockPlatformInterfaceMixin + implements VideoPlayerPlatform { + static const int _textureId = 0xffffffff; + + static StreamController _streamController = StreamController(); + static bool initialized = false; + static bool isPlaying = false; + + static void registerWith() { + VideoPlayerPlatform.instance = FakeVideoPlayerPlatform(); + } + + static void reset() { + _streamController.close(); + _streamController = StreamController(); + initialized = false; + isPlaying = false; + } + + @override + Future init() async {} + + @override + Future dispose(int textureId) async { + assert(initialized); + assert(textureId == _textureId); + initialized = false; + } + + @override + Future create(DataSource dataSource) async { + assert(!initialized); + initialized = true; + _streamController.add(VideoEvent( + eventType: VideoEventType.initialized, + duration: const Duration(seconds: 1), + size: const Size(0, 0), + rotationCorrection: 0, + )); + return _textureId; + } + + @override + Stream videoEventsFor(int textureId) { + assert(textureId == _textureId); + return _streamController.stream; + } + + @override + Future setLooping(int textureId, bool looping) async { + assert(textureId == _textureId); + assert(!looping); + } + + @override + Future play(int textureId) async { + assert(textureId == _textureId); + isPlaying = true; + _streamController.add(VideoEvent( + eventType: VideoEventType.isPlayingStateUpdate, + isPlaying: true, + )); + } + + @override + Future pause(int textureId) async { + assert(textureId == _textureId); + isPlaying = false; + _streamController.add(VideoEvent( + eventType: VideoEventType.isPlayingStateUpdate, + isPlaying: false, + )); + } + + @override + Future setVolume(int textureId, double volume) async { + assert(textureId == _textureId); + } + + @override + Future setPlaybackSpeed(int textureId, double speed) async { + assert(textureId == _textureId); + } + + @override + Widget buildView(int textureId) { + assert(textureId == _textureId); + return const SizedBox(width: 100, height: 100); + } +} + +void main() { + TestZulipBinding.ensureInitialized(); + + group("VideoLightboxPage", () { + FakeVideoPlayerPlatform.registerWith(); + + Future setupPage(WidgetTester tester, { + required Uri videoSrc, + }) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + addTearDown(FakeVideoPlayerPlatform.reset); + + await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp( + localizationsDelegates: ZulipLocalizations.localizationsDelegates, + supportedLocales: ZulipLocalizations.supportedLocales, + home: PerAccountStoreWidget( + accountId: eg.selfAccount.id, + child: VideoLightboxPage( + routeEntranceAnimation: kAlwaysCompleteAnimation, + message: eg.streamMessage(), + src: videoSrc))))); + await tester.pumpAndSettle(); + } + + testWidgets('shows a VideoPlayer, and video is playing', (tester) async { + await setupPage(tester, videoSrc: Uri.parse('https://a/b.mp4')); + + check(FakeVideoPlayerPlatform.initialized).isTrue(); + check(FakeVideoPlayerPlatform.isPlaying).isTrue(); + + await tester.ensureVisible(find.byType(VideoPlayer)); + }); + + testWidgets('toggles between play and pause', (tester) async { + await setupPage(tester, videoSrc: Uri.parse('https://a/b.mp4')); + check(FakeVideoPlayerPlatform.isPlaying).isTrue(); + + await tester.tap(find.byIcon(Icons.pause_circle_rounded)); + check(FakeVideoPlayerPlatform.isPlaying).isFalse(); + + // re-render to update player controls + await tester.pump(); + + await tester.tap(find.byIcon(Icons.play_circle_rounded)); + check(FakeVideoPlayerPlatform.isPlaying).isTrue(); + }); + }); +} From 33b97efdb40d369ccbb462b30d05d8b2d63dcff4 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 6 May 2024 12:54:16 -0700 Subject: [PATCH 11/11] dialog [nfc]: Disable barrierDismissible on custom onDismiss Based on the reasoning at the one place we're using either of these options so far, it seems like we'll always want them coupled this way. (At least until some other reason comes along, and in that case we can revise this logic again.) --- lib/widgets/dialog.dart | 11 +++++++---- lib/widgets/lightbox.dart | 7 +------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index edab8ebdbf..cdb027441c 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -19,19 +19,22 @@ Future showErrorDialog({ required BuildContext context, required String title, String? message, - bool barrierDismissible = true, - VoidCallback? onContinue, + VoidCallback? onDismiss, }) { final zulipLocalizations = ZulipLocalizations.of(context); return showDialog( context: context, - barrierDismissible: barrierDismissible, + // `showDialog` doesn't take an `onDismiss`, so dismissing via the barrier + // always causes the default dismiss behavior of popping just this route. + // When we want a non-default `onDismiss`, disable that. + // TODO(upstream): add onDismiss to showDialog, passing through to [ModalBarrier.onDismiss] + barrierDismissible: onDismiss == null, builder: (BuildContext context) => AlertDialog( title: Text(title), content: message != null ? SingleChildScrollView(child: Text(message)) : null, actions: [ TextButton( - onPressed: onContinue ?? () => Navigator.pop(context), + onPressed: onDismiss ?? () => Navigator.pop(context), child: _dialogActionText(zulipLocalizations.errorDialogContinue)), ])); } diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 818437cb28..da6b5bc185 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -400,12 +400,7 @@ class _VideoLightboxPageState extends State with PerAccountSt context: context, title: zulipLocalizations.errorDialogTitle, message: zulipLocalizations.errorVideoPlayerFailed, - // To avoid showing the disabled video lightbox for the unnsupported - // video, we make sure user doesn't reach there by dismissing the dialog - // by clicking around it, user must press the 'OK' button, which will - // take user back to content message list. - barrierDismissible: false, - onContinue: () { + onDismiss: () { Navigator.pop(context); // Pops the dialog Navigator.pop(context); // Pops the lightbox });