diff --git a/go/chat/livelocation_test.go b/go/chat/livelocation_test.go index 459a8b0e3c70..eda8490ef58a 100644 --- a/go/chat/livelocation_test.go +++ b/go/chat/livelocation_test.go @@ -81,7 +81,7 @@ func (m *mockUnfurler) Prefetch(ctx context.Context, uid gregor1.UID, convID cha } func (m *mockUnfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, - msg chat1.MessageUnboxed, + msg chat1.MessageUnboxed, suppress []string, ) { require.True(m.t, msg.IsValid()) body := msg.Valid().MessageBody diff --git a/go/chat/maps/livelocation.go b/go/chat/maps/livelocation.go index 76e24d886e23..54dd586b89bf 100644 --- a/go/chat/maps/livelocation.go +++ b/go/chat/maps/livelocation.go @@ -225,7 +225,7 @@ func (l *LiveLocationTracker) updateMapUnfurl(ctx context.Context, t *locationTr unfurlDoneCh := make(chan struct{}, 10) outboxID := storage.GetOutboxIDFromURL(body, t.convID, newMsg) listenerID := l.G().NotifyRouter.AddListener(newUnfurlNotifyListener(l.G(), outboxID, unfurlDoneCh)) - l.G().Unfurler.UnfurlAndSend(ctx, l.uid, t.convID, newMsg) + l.G().Unfurler.UnfurlAndSend(ctx, l.uid, t.convID, newMsg, nil) select { case <-unfurlDoneCh: case <-time.After(time.Minute): diff --git a/go/chat/sender.go b/go/chat/sender.go index 653934c17696..ad09cc28c5dd 100644 --- a/go/chat/sender.go +++ b/go/chat/sender.go @@ -1337,9 +1337,10 @@ func (s *BlockingSender) Send(ctx context.Context, convID chat1.ConversationID, chat1.ChatActivitySource_LOCAL) } if conv.GetTopicType() == chat1.TopicType_CHAT { - // Unfurl + // Unfurl. the suppressed urls travel with the message on its outbox record, so a + // send that waited offline still honours what the sender dismissed go s.G().Unfurler.UnfurlAndSend(globals.BackgroundChatCtx(ctx, s.G()), boxed.ClientHeader.Sender, - convID, unboxedMsg) + convID, unboxedMsg, sendOpts.GetUnfurlSuppress()) // Start tracking any live location sends if unboxedMsg.IsValid() && unboxedMsg.GetMessageType() == chat1.MessageType_TEXT && unboxedMsg.Valid().MessageBody.Text().LiveLocation != nil { diff --git a/go/chat/server.go b/go/chat/server.go index 9e3f9ac1c931..40410126f538 100644 --- a/go/chat/server.go +++ b/go/chat/server.go @@ -1006,6 +1006,7 @@ func (h *Server) PostTextNonblock(ctx context.Context, arg chat1.PostTextNonbloc } var parg chat1.PostLocalNonblockArg + parg.UnfurlSuppress = arg.UnfurlSuppress parg.SessionID = arg.SessionID parg.ClientPrev = arg.ClientPrev parg.ConversationID = arg.ConversationID @@ -1198,14 +1199,18 @@ func (h *Server) PostLocalNonblock(ctx context.Context, arg chat1.PostLocalNonbl // Create non block sender var prepareOpts chat1.SenderPrepareOptions + var sendOpts chat1.SenderSendOptions sender := NewBlockingSender(h.G(), h.boxer, h.remoteClient) nonblockSender := NewNonblockingSender(h.G(), sender) prepareOpts.ReplyTo = arg.ReplyTo + // rides the outbox record, so a message that waits offline still knows which urls the + // sender dismissed by the time it actually goes out + sendOpts.UnfurlSuppress = arg.UnfurlSuppress if arg.Msg.ClientHeader.Conv.TopicType == chat1.TopicType_NONE { arg.Msg.ClientHeader.Conv.TopicType = chat1.TopicType_CHAT } obid, _, err := nonblockSender.Send(ctx, arg.ConversationID, arg.Msg, arg.ClientPrev, arg.OutboxID, - nil, &prepareOpts) + &sendOpts, &prepareOpts) if err != nil { return res, fmt.Errorf("PostLocalNonblock: unable to send message: err: %s", err.Error()) } @@ -1611,6 +1616,16 @@ func (h *Server) UpdateUnsentText(ctx context.Context, arg chat1.UpdateUnsentTex return nil } +func (h *Server) UnfurlPreviewLocal(ctx context.Context, arg chat1.UnfurlPreviewLocalArg) (res []chat1.UnfurlPreviewInfo, err error) { + ctx = globals.ChatCtx(ctx, h.G(), keybase1.TLFIdentifyBehavior_CHAT_GUI, nil, h.identNotifier) + defer h.Trace(ctx, &err, "UnfurlPreviewLocal")() + uid, err := utils.AssertLoggedInUID(ctx, h.G()) + if err != nil { + return nil, err + } + return h.G().Unfurler.PreviewURLs(ctx, uid, arg.ConvID, arg.Text), nil +} + func (h *Server) UpdateTyping(ctx context.Context, arg chat1.UpdateTypingArg) (err error) { var identBreaks []keybase1.TLFIdentifyFailure ctx = globals.ChatCtx(ctx, h.G(), keybase1.TLFIdentifyBehavior_CHAT_GUI, @@ -2666,7 +2681,10 @@ func (h *Server) ResolveUnfurlPrompt(ctx context.Context, arg chat1.ResolveUnfur if len(msgs) != 1 { return errors.New("message not found") } - h.G().Unfurler.UnfurlAndSend(ctx, uid, arg.ConvID, msgs[0]) + // no suppress list on this pass: the message is already sent, so its outbox record + // is gone. urls dismissed at send time were marked then, and UnfurlAndSend reads + // those markers, so a dismissal still holds here + h.G().Unfurler.UnfurlAndSend(ctx, uid, arg.ConvID, msgs[0], nil) return nil } atyp, err := arg.Result.ActionType() diff --git a/go/chat/types/interfaces.go b/go/chat/types/interfaces.go index b19baf5f3901..cff757760b2e 100644 --- a/go/chat/types/interfaces.go +++ b/go/chat/types/interfaces.go @@ -504,8 +504,10 @@ type WhitelistExemption interface { type Unfurler interface { UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, - msg chat1.MessageUnboxed) + msg chat1.MessageUnboxed, suppress []string) Prefetch(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, msgText string) int + PreviewURLs(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, + text string) []chat1.UnfurlPreviewInfo Status(ctx context.Context, outboxID chat1.OutboxID) (UnfurlerTaskStatus, *chat1.UnfurlResult, error) Retry(ctx context.Context, outboxID chat1.OutboxID) Complete(ctx context.Context, outboxID chat1.OutboxID) diff --git a/go/chat/types/types.go b/go/chat/types/types.go index c9121540a9ef..ef7c7e36414e 100644 --- a/go/chat/types/types.go +++ b/go/chat/types/types.go @@ -581,13 +581,19 @@ type DummyUnfurler struct{} var _ Unfurler = (*DummyUnfurler)(nil) func (d DummyUnfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, - msg chat1.MessageUnboxed) { + msg chat1.MessageUnboxed, suppress []string) { } func (d DummyUnfurler) Prefetch(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, msgText string) int { return 0 } +func (d DummyUnfurler) PreviewURLs(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, + text string, +) []chat1.UnfurlPreviewInfo { + return nil +} + func (d DummyUnfurler) Status(ctx context.Context, outboxID chat1.OutboxID) (UnfurlerTaskStatus, *chat1.UnfurlResult, error) { return UnfurlerTaskStatusFailed, nil, nil } diff --git a/go/chat/unfurl/cache.go b/go/chat/unfurl/cache.go index e45cb8d990fc..b474e8a3b2a0 100644 --- a/go/chat/unfurl/cache.go +++ b/go/chat/unfurl/cache.go @@ -21,18 +21,24 @@ type cacheItem struct { type unfurlCache struct { sync.Mutex - cache *lru.Cache - clock clockwork.Clock + cache *lru.Cache + clock clockwork.Clock + lifetime time.Duration } func newUnfurlCache() *unfurlCache { + return newUnfurlCacheWithLifetime(defaultCacheLifetime) +} + +func newUnfurlCacheWithLifetime(lifetime time.Duration) *unfurlCache { cache, err := lru.New(defaultCacheSize) if err != nil { panic(err) } return &unfurlCache{ - cache: cache, - clock: clockwork.NewRealClock(), + cache: cache, + clock: clockwork.NewRealClock(), + lifetime: lifetime, } } @@ -40,9 +46,8 @@ func (c *unfurlCache) setClock(clock clockwork.Clock) { c.clock = clock } -// get determines if the item is in the cache and newer than 10 -// minutes. We don't want to cache this value indefinitely in case the page -// content changes. +// get determines if the item is in the cache and newer than the cache's lifetime. We +// don't want to cache this value indefinitely in case the page content changes. func (c *unfurlCache) get(key string) (res cacheItem, ok bool) { c.Lock() defer c.Unlock() @@ -55,7 +60,7 @@ func (c *unfurlCache) get(key string) (res cacheItem, ok bool) { if !ok { return res, false } - valid := c.clock.Now().Sub(cacheItem.ctime.Time()) <= defaultCacheLifetime + valid := c.clock.Now().Sub(cacheItem.ctime.Time()) <= c.lifetime if !valid { c.cache.Remove(key) } diff --git a/go/chat/unfurl/unfurler.go b/go/chat/unfurl/unfurler.go index 34e128dde240..a13cf6595a57 100644 --- a/go/chat/unfurl/unfurler.go +++ b/go/chat/unfurl/unfurler.go @@ -7,12 +7,15 @@ import ( "net" "net/url" "sync" + "time" + "github.com/golang/groupcache/singleflight" "github.com/keybase/client/go/chat/attachments" "github.com/keybase/client/go/chat/globals" "github.com/keybase/client/go/chat/s3" "github.com/keybase/client/go/chat/storage" "github.com/keybase/client/go/chat/types" + "github.com/keybase/client/go/chat/unfurl/display" "github.com/keybase/client/go/chat/utils" "github.com/keybase/client/go/libkb" "github.com/keybase/client/go/protocol/chat1" @@ -46,6 +49,11 @@ type UnfurlMessageSender interface { msg chat1.MessagePlaintext, clientPrev chat1.MessageID, outboxID chat1.OutboxID) (chat1.OutboxID, error) } +const ( + maxConcurrentPreviewScrapes = 4 + previewFailureLifetime = 30 * time.Second +) + type Unfurler struct { sync.Mutex prefetchLock sync.Mutex @@ -53,11 +61,23 @@ type Unfurler struct { utils.DebugLabeler unfurlMap map[string]bool - extractor *Extractor - scraper *Scraper - packager *Packager - settings *Settings - sender UnfurlMessageSender + // collapses concurrent preview scrapes of the same url. Prefetch gets this for free + // from prefetchLock, which serializes it; PreviewURLs is a synchronous rpc that can be + // called again while an earlier call is still in flight, which the composer does + // whenever the user edits a link before its fetch comes back + previewGroup singleflight.Group + // the composer scrapes on every debounced edit, and each edit of a url is a different + // key, so the singleflight above collapses nothing across them. these bound what an + // editing session can leave running: a slot limit on the detached scrapes, and a short + // memory of the urls that just failed so a dead link is not re-fetched per keystroke. + // short, because a url that starts working has to become a card again + previewSem chan struct{} + previewFailures *unfurlCache + extractor *Extractor + scraper *Scraper + packager *Packager + settings *Settings + sender UnfurlMessageSender // testing unfurlCh chan *chat1.Unfurl @@ -74,14 +94,16 @@ func NewUnfurler(g *globals.Context, store attachments.Store, s3signer s3.Signer packager := NewPackager(g, store, s3signer, ri) settings := NewSettings(g, storage) return &Unfurler{ - Contextified: globals.NewContextified(g), - DebugLabeler: utils.NewDebugLabeler(g.ExternalG(), "Unfurler", false), - unfurlMap: make(map[string]bool), - extractor: extractor, - scraper: scraper, - packager: packager, - settings: settings, - sender: sender, + previewSem: make(chan struct{}, maxConcurrentPreviewScrapes), + previewFailures: newUnfurlCacheWithLifetime(previewFailureLifetime), + Contextified: globals.NewContextified(g), + DebugLabeler: utils.NewDebugLabeler(g.ExternalG(), "Unfurler", false), + unfurlMap: make(map[string]bool), + extractor: extractor, + scraper: scraper, + packager: packager, + settings: settings, + sender: sender, } } @@ -115,6 +137,14 @@ func (u *Unfurler) statusKey(outboxID chat1.OutboxID) libkb.DbKey { } } +func (u *Unfurler) suppressedKey(outboxID chat1.OutboxID) libkb.DbKey { + return libkb.DbKey{ + Typ: libkb.DBUnfurler, + // not "s|", which is statusKey + Key: fmt.Sprintf("sup|%s", outboxID), + } +} + func (u *Unfurler) taskKey(outboxID chat1.OutboxID) libkb.DbKey { return libkb.DbKey{ Typ: libkb.DBUnfurler, @@ -234,8 +264,37 @@ func (u *Unfurler) makeBaseUnfurlMessage(ctx context.Context, fromMsg chat1.Mess return msg, nil } +// markSuppressed records that this URL was dismissed for this message, keyed by the same +// deterministic per-URL outbox ID the unfurl task would use. UnfurlAndSend runs again +// whenever the user resolves an unfurl prompt for another URL in the same message, and that +// pass carries no suppress list, so the marker is what keeps the dismissal honoured, on a +// clock or not. +// +// Nothing deletes these. Complete() clears the task and status keys for a URL that actually +// unfurled, but a suppressed URL never gets a task, so it has no such moment: the marker +// would have to be cleaned when the message itself goes away, which the unfurler is not +// told about. Each is one bool per dismissed URL per message, so the footprint is small and +// grows only with links the user explicitly declined. +func (u *Unfurler) markSuppressed(ctx context.Context, outboxID chat1.OutboxID) error { + return u.G().GetKVStore().PutObj(u.suppressedKey(outboxID), nil, true) +} + +func (u *Unfurler) isSuppressed(ctx context.Context, outboxID chat1.OutboxID) bool { + var found bool + ok, err := u.G().GetKVStore().GetInto(&found, u.suppressedKey(outboxID)) + if err != nil { + u.Debug(ctx, "isSuppressed: failed to read: %s", err) + return false + } + return ok && found +} + +// UnfurlAndSend unfurls the URLs in msg. suppress carries the URLs the sender chose not to +// unfurl; it arrives from the message's outbox record on the send that posts the message, +// and is empty on later passes over the same message (resolving an unfurl prompt calls this +// again), which read the marker left behind by the first pass instead. func (u *Unfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, - msg chat1.MessageUnboxed, + msg chat1.MessageUnboxed, suppress []string, ) { defer u.Trace(ctx, nil, "UnfurlAndSend")() // early out for errors @@ -248,6 +307,10 @@ func (u *Unfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID ch if len(hits) == 0 { return } + suppressed := make(map[string]bool, len(suppress)) + for _, url := range suppress { + suppressed[url] = true + } // get a map for all the URLs we have already unfurled prevUnfurled := make(map[string]bool) for _, u := range msg.Valid().Unfurls { @@ -260,6 +323,23 @@ func (u *Unfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID ch continue } prevUnfurled[hit.URL] = true // only one action per unique URL + // checked for every hit type, not just the unfurl one: a queued message can sit for + // a long time, and the whitelist it is classified against is read at send time, so a + // url the sender dismissed may have become a prompt hit in the meantime. prompting + // for a link they declined is the same broken promise as unfurling it + urlOutboxID := storage.GetOutboxIDFromURL(hit.URL, convID, msg) + marked := u.isSuppressed(ctx, urlOutboxID) + if suppressed[hit.URL] || marked { + u.Debug(ctx, "UnfurlAndSend: skipping suppressed URL: outboxID: %s", urlOutboxID) + if !marked { + // remember it: this runs again when the user resolves a prompt for another + // url in the same message, and that pass has no suppress list of its own + if err := u.markSuppressed(ctx, urlOutboxID); err != nil { + u.Debug(ctx, "UnfurlAndSend: failed to mark suppressed: %s", err) + } + } + continue + } switch hit.Typ { case ExtractorHitPrompt: domain, err := GetDomain(hit.URL) @@ -269,7 +349,7 @@ func (u *Unfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID ch } u.G().ActivityNotifier.PromptUnfurl(ctx, uid, convID, msg.GetMessageID(), domain) case ExtractorHitUnfurl: - outboxID := storage.GetOutboxIDFromURL(hit.URL, convID, msg) + outboxID := urlOutboxID if _, err := u.getTask(ctx, outboxID); err == nil { u.Debug(ctx, "UnfurlAndSend: skipping URL hit, task exists: outboxID: %s", outboxID) continue @@ -323,7 +403,13 @@ func (u *Unfurler) Prefetch(ctx context.Context, uid gregor1.UID, convID chat1.C } prevUnfurled[hit.URL] = true // only one action per unique URL if hit.Typ == ExtractorHitUnfurl { - if _, err := u.scrapeAndPackage(ctx, uid, convID, hit.URL); err != nil { + // through the same singleflight as the preview: UpdateUnsentText prefetches the + // text the composer is previewing, so the two would otherwise scrape it twice + key := previewScrapeKey(uid, convID, hit.URL) + _, err := u.previewGroup.Do(key, func() (any, error) { + return u.scrapeAndPackage(ctx, uid, convID, hit.URL) + }) + if err != nil { u.Debug(ctx, "Prefetch: unable to scrapeAndPackge: %s", err) } else { numPrefetched++ @@ -333,6 +419,141 @@ func (u *Unfurler) Prefetch(ctx context.Context, uid gregor1.UID, convID chat1.C return numPrefetched } +// previewable reports whether an unfurl can be shown in the pre-send preview. only generic +// unfurls render there, and a map unfurl is a generic unfurl with MapInfo set, which the +// message view itself refuses to render. the frontend filters on the same rule, so keep the +// two in step. +func previewable(unfurl chat1.Unfurl) bool { + typ, err := unfurl.UnfurlType() + if err != nil || typ != chat1.UnfurlType_GENERIC { + return false + } + return unfurl.Generic().MapInfo == nil +} + +// %x, not %s: uid and convID are raw bytes and can contain the separator +func previewScrapeKey(uid gregor1.UID, convID chat1.ConversationID, url string) string { + return fmt.Sprintf("%x:%x:%s", uid, convID, url) +} + +// previewScrape scrapes and packages one url, collapsing concurrent calls for the same +// url into a single scrape. +// +// the shared scrape deliberately does not run on the calling context. the composer calls +// PreviewURLs again on every edit and abandons the call in flight, so whichever caller +// happens to win the singleflight is also the one most likely to go away: running the +// scrape on its context would fail every other caller waiting on the same url. callers +// still honour their own cancellation, they just stop waiting rather than kill the work. +func (u *Unfurler) previewScrape(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, + url string, +) (chat1.Unfurl, error) { + key := previewScrapeKey(uid, convID, url) + if item, ok := u.previewFailures.get(key); ok { + if err, ok := item.data.(string); ok { + return chat1.Unfurl{}, errors.New(err) + } + } + // keeps the caller's log tags and identify behaviour and drops only the cancellation. + // the scraper bounds the work with its own request timeout + scrapeCtx := context.WithoutCancel(ctx) + type scrapeRes struct { + unfurl chat1.Unfurl + err error + } + ch := make(chan scrapeRes, 1) + go func() { + u.previewSem <- struct{}{} + defer func() { <-u.previewSem }() + scraped, err := u.previewGroup.Do(key, func() (any, error) { + return u.scrapeAndPackage(scrapeCtx, uid, convID, url) + }) + if err != nil { + u.previewFailures.put(key, err.Error()) + ch <- scrapeRes{err: err} + return + } + unfurl, ok := scraped.(chat1.Unfurl) + if !ok { + ch <- scrapeRes{err: fmt.Errorf("unexpected scrape result type: %T", scraped)} + return + } + ch <- scrapeRes{unfurl: unfurl} + }() + select { + case res := <-ch: + return res.unfurl, res.err + case <-ctx.Done(): + return chat1.Unfurl{}, ctx.Err() + } +} + +// carded reports whether a url could get a preview card at all, from the domain alone. +// previewable answers the same question from a scraped unfurl, but a failed scrape has no +// unfurl to ask about, and reporting one of these as un-previewable would suppress an +// unfurl the send's own retries would have landed. keep the two rules in step: this is the +// part of previewable that is knowable before the scrape. +func carded(url string) bool { + domain, err := GetDomain(url) + if err != nil { + return true + } + return ClassifyDomain(domain) == chat1.UnfurlType_GENERIC +} + +// PreviewURLs scrapes and packages the whitelisted URLs in text and returns display-ready +// unfurls, so a client can show a preview before sending. Only generic unfurls carry a +// display; a url that cannot be previewed at all comes back with a nil unfurl so the client +// can suppress it on send. +// +// URLs whose domain is not whitelisted are left out entirely: the send prompts for those, +// and the prompt is itself the chance to decline, so there is nothing for the composer to +// offer and nothing to suppress. +func (u *Unfurler) PreviewURLs(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, + text string, +) (res []chat1.UnfurlPreviewInfo) { + defer u.Trace(ctx, nil, "PreviewURLs")() + hits, err := u.extractor.Extract(ctx, uid, convID, 0, text, u.settings) + if err != nil { + u.Debug(ctx, "PreviewURLs: failed to extract: %s", err) + return nil + } + seen := make(map[string]bool) + for _, hit := range hits { + if hit.Typ != ExtractorHitUnfurl || seen[hit.URL] { + continue + } + seen[hit.URL] = true + unfurl, err := u.previewScrape(ctx, uid, convID, hit.URL) + if err != nil { + u.Debug(ctx, "PreviewURLs: unable to scrapeAndPackage: %s", err) + if ctx.Err() != nil { + return nil + } + if !carded(hit.URL) { + continue + } + // reported rather than dropped: UnfurlAndSend would still queue this url and + // retry it in the background for minutes after the send, so a card the user + // never saw could land in the sent message with no way to have declined it. + // telling the client makes it suppress the url instead + res = append(res, chat1.UnfurlPreviewInfo{Url: hit.URL}) + continue + } + if !previewable(unfurl) { + // a giphy or a map: no card in the composer, but the send unfurls it the same + // as it always has, so this is not something the client should suppress + continue + } + disp, err := display.DisplayUnfurl(ctx, u.G().AttachmentURLSrv, convID, unfurl) + if err != nil { + u.Debug(ctx, "PreviewURLs: failed to display: %s", err) + continue + } + res = append(res, chat1.UnfurlPreviewInfo{Url: hit.URL, Unfurl: &disp}) + } + return res +} + func (u *Unfurler) checkAndSetUnfurling(ctx context.Context, outboxID chat1.OutboxID) (inprogress bool) { u.Lock() defer u.Unlock() diff --git a/go/chat/unfurl/unfurler_test.go b/go/chat/unfurl/unfurler_test.go index 596da8a9165d..783ec0a419c3 100644 --- a/go/chat/unfurl/unfurler_test.go +++ b/go/chat/unfurl/unfurler_test.go @@ -1,19 +1,27 @@ package unfurl import ( + "bytes" "context" "fmt" + "io" + "net/http" + "os" + "path/filepath" "strings" + "sync/atomic" "testing" "time" "github.com/keybase/client/go/chat/attachments" "github.com/keybase/client/go/chat/globals" + "github.com/keybase/client/go/chat/storage" "github.com/keybase/client/go/chat/types" "github.com/keybase/client/go/externalstest" "github.com/keybase/client/go/libkb" "github.com/keybase/client/go/protocol/chat1" "github.com/keybase/client/go/protocol/gregor1" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -109,7 +117,7 @@ func TestUnfurler(t *testing.T) { numPrefetched := unfurler.Prefetch(context.TODO(), uid, convID, msgBody) require.Equal(t, 0, numPrefetched) - unfurler.UnfurlAndSend(context.TODO(), uid, convID, fromMsg) + unfurler.UnfurlAndSend(context.TODO(), uid, convID, fromMsg, nil) select { case <-sender.ch: require.Fail(t, "no send here") @@ -128,7 +136,7 @@ func TestUnfurler(t *testing.T) { require.Equal(t, 1, numPrefetched) for range 5 { - unfurler.UnfurlAndSend(context.TODO(), uid, convID, fromMsg) + unfurler.UnfurlAndSend(context.TODO(), uid, convID, fromMsg, nil) } var outboxID chat1.OutboxID select { @@ -176,3 +184,383 @@ func TestUnfurler(t *testing.T) { require.ErrorAs(t, err, new(libkb.NotFoundError)) require.Equal(t, types.UnfurlerTaskStatusFailed, status) } + +func TestUnfurlerPreviewURLs(t *testing.T) { + tc := externalstest.SetupTest(t, "unfurler", 0) + defer tc.Cleanup() + g := globals.NewContext(tc.G, &globals.ChatContext{}) + + store := attachments.NewStoreTesting(g, nil) + s3signer := &ptsigner{} + g.ActivityNotifier = makeDummyActivityNotifier() + g.MessageDeliverer = dummyDeliverer{} + g.AttachmentURLSrv = types.DummyAttachmentHTTPSrv{} + sender := makeDummySender() + ri := func() chat1.RemoteInterface { return paramsRemote{} } + storage := newMemConversationBackedStorage() + unfurler := NewUnfurler(g, store, s3signer, storage, sender, ri) + + uid := gregor1.UID([]byte{0, 1}) + convID := chat1.ConversationID([]byte{0, 1, 2}) + srv := createTestCaseHTTPSrv(t) + addr := srv.Start() + defer srv.Stop() + + url := fmt.Sprintf("http://%s/?name=%s", addr, "wsj0.html") + require.NoError(t, unfurler.WhitelistAdd(context.TODO(), uid, "127.0.0.1")) + + res := unfurler.PreviewURLs(context.TODO(), uid, convID, "check this out "+url) + require.Len(t, res, 1) + require.Equal(t, url, res[0].Url) + require.NotNil(t, res[0].Unfurl) + typ, err := res[0].Unfurl.UnfurlType() + require.NoError(t, err) + require.Equal(t, chat1.UnfurlType_GENERIC, typ) + require.NotEmpty(t, res[0].Unfurl.Generic().Title) + + // duplicate URLs collapse to one entry + res = unfurler.PreviewURLs(context.TODO(), uid, convID, url+" and again "+url) + require.Len(t, res, 1) + + // text with no links does no work + require.Empty(t, unfurler.PreviewURLs(context.TODO(), uid, convID, "no links here")) +} + +// a url the scraper cannot fetch still comes back, with no unfurl on it: the send path +// would queue an unfurl for it anyway, so the client needs to know to suppress it +func TestUnfurlerPreviewURLsScrapeFailure(t *testing.T) { + tc := externalstest.SetupTest(t, "unfurler", 0) + defer tc.Cleanup() + g := globals.NewContext(tc.G, &globals.ChatContext{}) + + store := attachments.NewStoreTesting(g, nil) + s3signer := &ptsigner{} + g.ActivityNotifier = makeDummyActivityNotifier() + g.MessageDeliverer = dummyDeliverer{} + g.AttachmentURLSrv = types.DummyAttachmentHTTPSrv{} + sender := makeDummySender() + ri := func() chat1.RemoteInterface { return paramsRemote{} } + storage := newMemConversationBackedStorage() + unfurler := NewUnfurler(g, store, s3signer, storage, sender, ri) + + uid := gregor1.UID([]byte{0, 1}) + convID := chat1.ConversationID([]byte{0, 1, 2}) + srv := newDummyHTTPSrv(t, func(w http.ResponseWriter, r *http.Request) { + // what wsj.com does to the scraper + w.WriteHeader(http.StatusUnauthorized) + }) + addr := srv.Start() + defer srv.Stop() + + url := fmt.Sprintf("http://%s/?name=%s", addr, "wsj0.html") + require.NoError(t, unfurler.WhitelistAdd(context.TODO(), uid, "127.0.0.1")) + + res := unfurler.PreviewURLs(context.TODO(), uid, convID, "check this out "+url) + require.Len(t, res, 1) + require.Equal(t, url, res[0].Url) + require.Nil(t, res[0].Unfurl) +} + +// the composer previews on every debounced edit, so a url that cannot be scraped would be +// re-fetched for as long as it sits in the text. the failure is remembered briefly instead +func TestUnfurlerPreviewURLsFailureNotRescraped(t *testing.T) { + tc := externalstest.SetupTest(t, "unfurler", 0) + defer tc.Cleanup() + g := globals.NewContext(tc.G, &globals.ChatContext{}) + + store := attachments.NewStoreTesting(g, nil) + s3signer := &ptsigner{} + g.ActivityNotifier = makeDummyActivityNotifier() + g.MessageDeliverer = dummyDeliverer{} + g.AttachmentURLSrv = types.DummyAttachmentHTTPSrv{} + sender := makeDummySender() + ri := func() chat1.RemoteInterface { return paramsRemote{} } + storage := newMemConversationBackedStorage() + unfurler := NewUnfurler(g, store, s3signer, storage, sender, ri) + + uid := gregor1.UID([]byte{0, 1}) + convID := chat1.ConversationID([]byte{0, 1, 2}) + var hits int32 + srv := newDummyHTTPSrv(t, func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&hits, 1) + w.WriteHeader(http.StatusUnauthorized) + }) + addr := srv.Start() + defer srv.Stop() + + url := fmt.Sprintf("http://%s/?name=%s", addr, "wsj0.html") + require.NoError(t, unfurler.WhitelistAdd(context.TODO(), uid, "127.0.0.1")) + + for i := 0; i < 3; i++ { + res := unfurler.PreviewURLs(context.TODO(), uid, convID, "check this out "+url) + require.Len(t, res, 1) + require.Nil(t, res[0].Unfurl, "a cached failure still reports the url for suppression") + } + require.Equal(t, int32(1), atomic.LoadInt32(&hits)) +} + +func makeTextMsgWithMsgID(msgBody string, outboxID chat1.OutboxID, msgID chat1.MessageID) chat1.MessageUnboxed { + return chat1.NewMessageUnboxedWithValid(chat1.MessageUnboxedValid{ + ClientHeader: chat1.MessageClientHeaderVerified{ + TlfName: "mike", + MessageType: chat1.MessageType_TEXT, + OutboxID: &outboxID, + }, + ServerHeader: chat1.MessageServerHeader{ + // the suppression key derives from this, not from the outbox id, so tests that + // mean "a different message" have to vary it + MessageID: msgID, + }, + MessageBody: chat1.NewMessageBodyWithText(chat1.MessageText{ + Body: msgBody, + }), + }) +} + +func TestUnfurlerSuppress(t *testing.T) { + tc := externalstest.SetupTest(t, "unfurler", 0) + defer tc.Cleanup() + g := globals.NewContext(tc.G, &globals.ChatContext{}) + + store := attachments.NewStoreTesting(g, nil) + s3signer := &ptsigner{} + g.ActivityNotifier = makeDummyActivityNotifier() + g.MessageDeliverer = dummyDeliverer{} + sender := makeDummySender() + ri := func() chat1.RemoteInterface { return paramsRemote{} } + memStorage := newMemConversationBackedStorage() + unfurler := NewUnfurler(g, store, s3signer, memStorage, sender, ri) + + uid := gregor1.UID([]byte{0, 1}) + convID := chat1.ConversationID([]byte{0, 1, 2}) + srv := createTestCaseHTTPSrv(t) + addr := srv.Start() + defer srv.Stop() + + url := fmt.Sprintf("http://%s/?name=%s", addr, "wsj0.html") + require.NoError(t, unfurler.WhitelistAdd(context.TODO(), uid, "127.0.0.1")) + + outboxID, err := storage.NewOutboxID() + require.NoError(t, err) + msg := makeTextMsgWithMsgID("check out this link! "+url, outboxID, 4) + + unfurler.UnfurlAndSend(context.TODO(), uid, convID, msg, []string{url}) + select { + case <-sender.ch: + require.Fail(t, "should not have sent a suppressed unfurl") + case <-time.After(2 * time.Second): + } + + // resolving an unfurl prompt for another URL re-runs UnfurlAndSend on the same message + // with no suppress list of its own. the marker left by the first pass is what keeps the + // dismissal honoured, and it is not on a clock: however long the message waited to + // send, this pass must still skip the URL + unfurler.UnfurlAndSend(context.TODO(), uid, convID, msg, nil) + select { + case <-sender.ch: + require.Fail(t, "should not have sent a suppressed unfurl on the second pass") + case <-time.After(2 * time.Second): + } + + // the dismissal is scoped to that message: the same url sent again, undismissed, must + // still unfurl. the suppression key derives from the message id, so this needs a + // genuinely different message rather than just a different outbox id + laterMsg := makeTextMsgWithMsgID("sending it again "+url, outboxID, 5) + unfurler.UnfurlAndSend(context.TODO(), uid, convID, laterMsg, nil) + select { + case <-sender.ch: + case <-time.After(20 * time.Second): + require.Fail(t, "no unfurl message sent for the same url in a later message") + } +} + +func TestPreviewable(t *testing.T) { + generic := chat1.NewUnfurlWithGeneric(chat1.UnfurlGeneric{Title: "t"}) + require.True(t, previewable(generic)) + + // a map is a generic unfurl carrying MapInfo; the message view will not render one, + // so the preview must not either + mapped := chat1.NewUnfurlWithGeneric(chat1.UnfurlGeneric{ + Title: "here", + MapInfo: &chat1.UnfurlGenericMapInfo{IsLiveLocationDone: true}, + }) + require.False(t, previewable(mapped)) + + require.False(t, previewable(chat1.NewUnfurlWithYoutube(chat1.UnfurlYoutube{}))) + require.False(t, previewable(chat1.NewUnfurlWithGiphy(chat1.UnfurlGiphy{}))) +} + +func TestUnfurlerSuppressPrompt(t *testing.T) { + tc := externalstest.SetupTest(t, "unfurler", 0) + defer tc.Cleanup() + g := globals.NewContext(tc.G, &globals.ChatContext{}) + + store := attachments.NewStoreTesting(g, nil) + s3signer := &ptsigner{} + notifier := makeDummyActivityNotifier() + g.ActivityNotifier = notifier + g.MessageDeliverer = dummyDeliverer{} + sender := makeDummySender() + ri := func() chat1.RemoteInterface { return paramsRemote{} } + memStorage := newMemConversationBackedStorage() + unfurler := NewUnfurler(g, store, s3signer, memStorage, sender, ri) + + uid := gregor1.UID([]byte{0, 1}) + convID := chat1.ConversationID([]byte{0, 1, 2}) + srv := createTestCaseHTTPSrv(t) + addr := srv.Start() + defer srv.Stop() + + // no WhitelistAdd here, so this url classifies as a prompt hit rather than an unfurl + // one. a queued message is classified at send time, so a url the sender dismissed while + // the domain was whitelisted can arrive here as a prompt: prompting for a link they + // declined breaks the same promise as unfurling it + url := fmt.Sprintf("http://%s/?name=%s", addr, "wsj0.html") + outboxID, err := storage.NewOutboxID() + require.NoError(t, err) + msg := makeTextMsgWithMsgID("check out this link! "+url, outboxID, 4) + + unfurler.UnfurlAndSend(context.TODO(), uid, convID, msg, []string{url}) + select { + case n := <-notifier.ch: + require.Failf(t, "prompted for a suppressed URL", "domain: %s", n.domain) + case <-time.After(2 * time.Second): + } + + // the same url in a later message still prompts, so the check above is not passing + // because prompting is broken + unfurler.UnfurlAndSend(context.TODO(), uid, convID, + makeTextMsgWithMsgID("sending it again "+url, outboxID, 5), nil) + select { + case <-notifier.ch: + case <-time.After(20 * time.Second): + require.Fail(t, "no prompt for the unsuppressed URL") + } +} + +// an abandoned preview caller must not take the scrape down with it: the composer calls +// PreviewURLs again on every edit, so the caller that wins the singleflight is often the +// one that goes away first +func TestUnfurlerPreviewURLsCallerCancel(t *testing.T) { + tc := externalstest.SetupTest(t, "unfurler", 0) + defer tc.Cleanup() + g := globals.NewContext(tc.G, &globals.ChatContext{}) + + store := attachments.NewStoreTesting(g, nil) + s3signer := &ptsigner{} + g.ActivityNotifier = makeDummyActivityNotifier() + g.MessageDeliverer = dummyDeliverer{} + g.AttachmentURLSrv = types.DummyAttachmentHTTPSrv{} + sender := makeDummySender() + ri := func() chat1.RemoteInterface { return paramsRemote{} } + storage := newMemConversationBackedStorage() + unfurler := NewUnfurler(g, store, s3signer, storage, sender, ri) + + uid := gregor1.UID([]byte{0, 1}) + convID := chat1.ConversationID([]byte{0, 1, 2}) + + var scrapes int64 + started := make(chan struct{}, 1) + release := make(chan struct{}) + srv := newDummyHTTPSrv(t, func(w http.ResponseWriter, r *http.Request) { + name := r.URL.Query().Get("name") + if name == "wsj0.html" && atomic.AddInt64(&scrapes, 1) == 1 { + started <- struct{}{} + <-release + } + w.WriteHeader(200) + dat, err := os.ReadFile(filepath.Join("testcases", name)) + assert.NoError(t, err) + _, err = io.Copy(w, bytes.NewBuffer(dat)) + assert.NoError(t, err) + }) + addr := srv.Start() + defer srv.Stop() + + url := fmt.Sprintf("http://%s/?name=%s", addr, "wsj0.html") + require.NoError(t, unfurler.WhitelistAdd(context.TODO(), uid, "127.0.0.1")) + + // the first caller wins the singleflight and is then abandoned mid-scrape + firstCtx, cancelFirst := context.WithCancel(context.TODO()) + firstCh := make(chan []chat1.UnfurlPreviewInfo, 1) + go func() { firstCh <- unfurler.PreviewURLs(firstCtx, uid, convID, "check this out "+url) }() + select { + case <-started: + case <-time.After(20 * time.Second): + require.Fail(t, "scrape never started") + } + + // the second caller joins the same scrape, then the first one goes away + secondCh := make(chan []chat1.UnfurlPreviewInfo, 1) + go func() { secondCh <- unfurler.PreviewURLs(context.TODO(), uid, convID, "check this out "+url) }() + cancelFirst() + select { + case res := <-firstCh: + require.Empty(t, res, "a cancelled caller should not return a preview") + case <-time.After(20 * time.Second): + require.Fail(t, "cancelled caller never returned") + } + + close(release) + select { + case res := <-secondCh: + require.Len(t, res, 1, "the surviving caller lost its preview to the cancelled one") + require.Equal(t, url, res[0].Url) + require.NotNil(t, res[0].Unfurl) + require.NotEmpty(t, res[0].Unfurl.Generic().Title) + case <-time.After(20 * time.Second): + require.Fail(t, "surviving caller never returned") + } + require.Equal(t, int64(1), atomic.LoadInt64(&scrapes), "the two callers did not share one scrape") +} + +// only generic unfurls get a card, and a map is a generic unfurl the message view itself +// refuses to render. the frontend filters on the same rule +func TestUnfurlerPreviewable(t *testing.T) { + generic := chat1.UnfurlGeneric{Title: "t", Url: "u", SiteName: "s"} + require.True(t, previewable(chat1.NewUnfurlWithGeneric(generic))) + + withMap := generic + withMap.MapInfo = &chat1.UnfurlGenericMapInfo{} + require.False(t, previewable(chat1.NewUnfurlWithGeneric(withMap))) + + require.False(t, previewable(chat1.NewUnfurlWithGiphy(chat1.UnfurlGiphy{}))) + require.False(t, previewable(chat1.NewUnfurlWithYoutube(chat1.UnfurlYoutube{}))) +} + +// the rule for what gets no card is the one previewable uses on a scraped unfurl, read +// from the domain: a giphy short link classifies as giphy without being on the +// auto-whitelist, and suppressing it would lose an unfurl the send would have landed +func TestUnfurlerCarded(t *testing.T) { + require.True(t, carded("https://example.com/a")) + require.True(t, carded("not a url at all")) + require.False(t, carded("https://giphy.com/gifs/abc")) + require.False(t, carded("https://gph.is/2X9abc")) + require.False(t, carded(fmt.Sprintf("https://%s/?lat=1&lon=2&acc=3&done=true", types.MapsDomain))) +} + +// a giphy or a maps url gets no card either way, so a scrape failure on one must not be +// reported: suppressing it would lose an unfurl the send's own retries would have landed +func TestUnfurlerPreviewURLsAutoWhitelistFailureNotSuppressed(t *testing.T) { + tc := externalstest.SetupTest(t, "unfurler", 0) + defer tc.Cleanup() + g := globals.NewContext(tc.G, &globals.ChatContext{}) + + store := attachments.NewStoreTesting(g, nil) + s3signer := &ptsigner{} + g.ActivityNotifier = makeDummyActivityNotifier() + g.MessageDeliverer = dummyDeliverer{} + g.AttachmentURLSrv = types.DummyAttachmentHTTPSrv{} + sender := makeDummySender() + ri := func() chat1.RemoteInterface { return paramsRemote{} } + storage := newMemConversationBackedStorage() + unfurler := NewUnfurler(g, store, s3signer, storage, sender, ri) + + uid := gregor1.UID([]byte{0, 1}) + convID := chat1.ConversationID([]byte{0, 1, 2}) + // the maps domain is auto-whitelisted and scraped locally, so an unparseable coord + // fails the scrape without touching the network + url := fmt.Sprintf("https://%s/?lat=nope&lon=1&acc=1&done=true", types.MapsDomain) + + require.Empty(t, unfurler.PreviewURLs(context.TODO(), uid, convID, "check this out "+url)) +} diff --git a/go/protocol/chat1/extras.go b/go/protocol/chat1/extras.go index df39d55e7d2b..f4277c96ac20 100644 --- a/go/protocol/chat1/extras.go +++ b/go/protocol/chat1/extras.go @@ -3092,6 +3092,13 @@ func (o *SenderSendOptions) GetJoinMentionsAs() *ConversationMemberStatus { return o.JoinMentionsAs } +func (o *SenderSendOptions) GetUnfurlSuppress() []string { + if o == nil { + return nil + } + return o.UnfurlSuppress +} + func (c Coordinate) IsZero() bool { return c.Lat == 0 && c.Lon == 0 } diff --git a/go/protocol/chat1/local.go b/go/protocol/chat1/local.go index 66afa09eb337..16d40c1171d1 100644 --- a/go/protocol/chat1/local.go +++ b/go/protocol/chat1/local.go @@ -1757,6 +1757,7 @@ func (o SenderPrepareOptions) DeepCopy() SenderPrepareOptions { type SenderSendOptions struct { JoinMentionsAs *ConversationMemberStatus `codec:"joinMentionsAs,omitempty" json:"joinMentionsAs,omitempty"` + UnfurlSuppress []string `codec:"unfurlSuppress" json:"unfurlSuppress"` } func (o SenderSendOptions) DeepCopy() SenderSendOptions { @@ -1768,6 +1769,17 @@ func (o SenderSendOptions) DeepCopy() SenderSendOptions { tmp := x.DeepCopy() return &tmp })(o.JoinMentionsAs), + UnfurlSuppress: (func(x []string) []string { + if x == nil { + return nil + } + ret := make([]string, len(x)) + for i, v := range x { + vCopy := v + ret[i] = vCopy + } + return ret + })(o.UnfurlSuppress), } } @@ -5844,6 +5856,24 @@ func (o UnfurlPromptResult) DeepCopy() UnfurlPromptResult { } } +type UnfurlPreviewInfo struct { + Url string `codec:"url" json:"url"` + Unfurl *UnfurlDisplay `codec:"unfurl,omitempty" json:"unfurl,omitempty"` +} + +func (o UnfurlPreviewInfo) DeepCopy() UnfurlPreviewInfo { + return UnfurlPreviewInfo{ + Url: o.Url, + Unfurl: (func(x *UnfurlDisplay) *UnfurlDisplay { + if x == nil { + return nil + } + tmp := x.DeepCopy() + return &tmp + })(o.Unfurl), + } +} + type GalleryItemTyp int const ( @@ -6816,6 +6846,7 @@ type PostLocalNonblockArg struct { ReplyTo *MessageID `codec:"replyTo,omitempty" json:"replyTo,omitempty"` IdentifyBehavior keybase1.TLFIdentifyBehavior `codec:"identifyBehavior" json:"identifyBehavior"` SkipInChatPayments bool `codec:"skipInChatPayments" json:"skipInChatPayments"` + UnfurlSuppress []string `codec:"unfurlSuppress" json:"unfurlSuppress"` } type ForwardMessageArg struct { @@ -6847,6 +6878,7 @@ type PostTextNonblockArg struct { OutboxID *OutboxID `codec:"outboxID,omitempty" json:"outboxID,omitempty"` IdentifyBehavior keybase1.TLFIdentifyBehavior `codec:"identifyBehavior" json:"identifyBehavior"` EphemeralLifetime *gregor1.DurationSec `codec:"ephemeralLifetime,omitempty" json:"ephemeralLifetime,omitempty"` + UnfurlSuppress []string `codec:"unfurlSuppress" json:"unfurlSuppress"` } type PostDeleteNonblockArg struct { @@ -7234,6 +7266,11 @@ type SaveUnfurlSettingsArg struct { Whitelist []string `codec:"whitelist" json:"whitelist"` } +type UnfurlPreviewLocalArg struct { + ConvID ConversationID `codec:"convID" json:"convID"` + Text string `codec:"text" json:"text"` +} + type ToggleMessageCollapseArg struct { ConvID ConversationID `codec:"convID" json:"convID"` MsgID MessageID `codec:"msgID" json:"msgID"` @@ -7553,6 +7590,7 @@ type LocalInterface interface { ResolveUnfurlPrompt(context.Context, ResolveUnfurlPromptArg) error GetUnfurlSettings(context.Context) (UnfurlSettingsDisplay, error) SaveUnfurlSettings(context.Context, SaveUnfurlSettingsArg) error + UnfurlPreviewLocal(context.Context, UnfurlPreviewLocalArg) ([]UnfurlPreviewInfo, error) ToggleMessageCollapse(context.Context, ToggleMessageCollapseArg) error BulkAddToConv(context.Context, BulkAddToConvArg) error BulkAddToManyConvs(context.Context, BulkAddToManyConvsArg) error @@ -8777,6 +8815,21 @@ func LocalProtocol(i LocalInterface) rpc.Protocol { return }, }, + "unfurlPreviewLocal": { + MakeArg: func() any { + var ret [1]UnfurlPreviewLocalArg + return &ret + }, + Handler: func(ctx context.Context, args any) (ret any, err error) { + typedArgs, ok := args.(*[1]UnfurlPreviewLocalArg) + if !ok { + err = rpc.NewTypeError((*[1]UnfurlPreviewLocalArg)(nil), args) + return + } + ret, err = i.UnfurlPreviewLocal(ctx, typedArgs[0]) + return + }, + }, "toggleMessageCollapse": { MakeArg: func() any { var ret [1]ToggleMessageCollapseArg @@ -9921,6 +9974,11 @@ func (c LocalClient) SaveUnfurlSettings(ctx context.Context, __arg SaveUnfurlSet return } +func (c LocalClient) UnfurlPreviewLocal(ctx context.Context, __arg UnfurlPreviewLocalArg) (res []UnfurlPreviewInfo, err error) { + err = c.Cli.Call(ctx, "chat.1.local.unfurlPreviewLocal", []any{__arg}, &res, 15000*time.Millisecond) + return +} + func (c LocalClient) ToggleMessageCollapse(ctx context.Context, __arg ToggleMessageCollapseArg) (err error) { err = c.Cli.Call(ctx, "chat.1.local.toggleMessageCollapse", []any{__arg}, nil, 0*time.Millisecond) return diff --git a/protocol/avdl/chat1/local.avdl b/protocol/avdl/chat1/local.avdl index 3312374e5fbf..ee55421303bb 100644 --- a/protocol/avdl/chat1/local.avdl +++ b/protocol/avdl/chat1/local.avdl @@ -272,6 +272,9 @@ protocol local { record SenderSendOptions { union { null, ConversationMemberStatus } joinMentionsAs; + // urls the sender chose not to unfurl; rides the outbox record so a message that + // waits offline still carries them when it finally sends + array unfurlSuppress; } enum OutboxStateType { @@ -829,7 +832,7 @@ protocol local { OutboxID generateOutboxID(); @timeout_msec(30000) // 30 seconds - PostLocalNonblockRes postLocalNonblock(int sessionID, ConversationID conversationID, MessagePlaintext msg, MessageID clientPrev, union { null, OutboxID } outboxID, union { null, MessageID } replyTo, keybase1.TLFIdentifyBehavior identifyBehavior, boolean skipInChatPayments); + PostLocalNonblockRes postLocalNonblock(int sessionID, ConversationID conversationID, MessagePlaintext msg, MessageID clientPrev, union { null, OutboxID } outboxID, union { null, MessageID } replyTo, keybase1.TLFIdentifyBehavior identifyBehavior, boolean skipInChatPayments, array unfurlSuppress); record PostLocalNonblockRes { array rateLimits; OutboxID outboxID; @@ -841,7 +844,7 @@ protocol local { PostLocalNonblockRes forwardMessageNonblock(int sessionID, ConversationID srcConvID, ConversationID dstConvID, MessageID msgID, keybase1.TLFIdentifyBehavior identifyBehavior, string title); @timeout_msec(30000) // 30 seconds - PostLocalNonblockRes postTextNonblock(int sessionID, ConversationID conversationID, string tlfName, boolean tlfPublic, string body, MessageID clientPrev, union { null, MessageID } replyTo, union { null, OutboxID } outboxID, keybase1.TLFIdentifyBehavior identifyBehavior, union {null, gregor1.DurationSec} ephemeralLifetime); + PostLocalNonblockRes postTextNonblock(int sessionID, ConversationID conversationID, string tlfName, boolean tlfPublic, string body, MessageID clientPrev, union { null, MessageID } replyTo, union { null, OutboxID } outboxID, keybase1.TLFIdentifyBehavior identifyBehavior, union {null, gregor1.DurationSec} ephemeralLifetime, array unfurlSuppress); @timeout_msec(30000) // 30 seconds PostLocalNonblockRes postDeleteNonblock(ConversationID conversationID, string tlfName, boolean tlfPublic, MessageID supersedes,MessageID clientPrev, union { null, OutboxID } outboxID, keybase1.TLFIdentifyBehavior identifyBehavior); @@ -1243,6 +1246,16 @@ protocol local { UnfurlSettingsDisplay getUnfurlSettings(); void saveUnfurlSettings(UnfurlMode mode, array whitelist); + record UnfurlPreviewInfo { + string url; + // null when the url could not be scraped or packaged. the client suppresses those + // urls on send, so the preview it showed is exactly what the message unfurls + union { null, UnfurlDisplay } unfurl; + } + + @timeout_msec(15000) + array unfurlPreviewLocal(ConversationID convID, string text); + void toggleMessageCollapse(ConversationID convID, MessageID msgID, boolean collapse); void bulkAddToConv(ConversationID convID, array usernames); diff --git a/protocol/bin/enabled-calls.json b/protocol/bin/enabled-calls.json index 664a2f886ccb..35ebd092896e 100644 --- a/protocol/bin/enabled-calls.json +++ b/protocol/bin/enabled-calls.json @@ -147,6 +147,7 @@ "chat.1.local.toggleMessageCollapse": {"promise":true}, "chat.1.local.trackGiphySelect": {"promise":true}, "chat.1.local.unboxMobilePushNotification": {"promise":true}, + "chat.1.local.unfurlPreviewLocal": {"promise":true}, "chat.1.local.unpinMessage": {"promise":true}, "chat.1.local.updateTyping": {"promise":true}, "chat.1.local.updateUnsentText": {"promise":true}, diff --git a/protocol/json/chat1/local.json b/protocol/json/chat1/local.json index f8008d79a0b2..6188aa6d0cd8 100644 --- a/protocol/json/chat1/local.json +++ b/protocol/json/chat1/local.json @@ -996,6 +996,13 @@ "ConversationMemberStatus" ], "name": "joinMentionsAs" + }, + { + "type": { + "type": "array", + "items": "string" + }, + "name": "unfurlSuppress" } ] }, @@ -3672,6 +3679,23 @@ } ] }, + { + "type": "record", + "name": "UnfurlPreviewInfo", + "fields": [ + { + "type": "string", + "name": "url" + }, + { + "type": [ + null, + "UnfurlDisplay" + ], + "name": "unfurl" + } + ] + }, { "type": "enum", "name": "GalleryItemTyp", @@ -4642,6 +4666,13 @@ { "name": "skipInChatPayments", "type": "boolean" + }, + { + "name": "unfurlSuppress", + "type": { + "type": "array", + "items": "string" + } } ], "response": "PostLocalNonblockRes", @@ -4756,6 +4787,13 @@ null, "gregor1.DurationSec" ] + }, + { + "name": "unfurlSuppress", + "type": { + "type": "array", + "items": "string" + } } ], "response": "PostLocalNonblockRes", @@ -5918,6 +5956,23 @@ ], "response": null }, + "unfurlPreviewLocal": { + "request": [ + { + "name": "convID", + "type": "ConversationID" + }, + { + "name": "text", + "type": "string" + } + ], + "response": { + "type": "array", + "items": "UnfurlPreviewInfo" + }, + "timeout_msec": 15000 + }, "toggleMessageCollapse": { "request": [ { diff --git a/shared/chat/conversation/input-area/input-state.test.tsx b/shared/chat/conversation/input-area/input-state.test.tsx index ba6abdef4184..9eebd56b7cb3 100644 --- a/shared/chat/conversation/input-area/input-state.test.tsx +++ b/shared/chat/conversation/input-area/input-state.test.tsx @@ -4,14 +4,46 @@ import * as Message from '@/constants/chat/message' import type * as React from 'react' import * as T from '@/constants/types' import HiddenString from '@/util/hidden-string' -import {act, cleanup, renderHook} from '@testing-library/react' +import {act, cleanup, render, renderHook} from '@testing-library/react' import {notifyEngineActionListeners} from '@/engine/action-listener' import {resetAllStores} from '@/util/zustand' import {useCurrentUserState} from '@/stores/current-user' -import {ConversationInputProvider, useConversationInput} from './input-state' +import Input from './normal' +import type {PlatformInputProps} from './normal/input.shared' +import {ConversationInputProvider, useConversationInput, type ConversationInputState} from './input-state' import {ConversationThreadProvider, useConversationThreadActions} from '../thread-context' +import {suppressedURLsOf, takeSuppressSnapshot, useUnfurlPreviewState} from '../unfurl-preview-state' + +const getSuppressedURLs = (c: T.Chat.ConversationIDKey) => suppressedURLsOf(takeSuppressSnapshot(c)) let mockRouteParams: Record = {} +// jest.mock factories may only close over mock-prefixed names +let mockOnClear: (() => void) | undefined +let mockPlatformInputProps: PlatformInputProps | undefined +// stand in for the real composer input: it only has to hand back a ref whose clear() +// fires onChangeText('') the way the desktop input does, which is what races the send +jest.mock('./normal/input', () => ({ + __esModule: true, + default: function MockPlatformInput(p: PlatformInputProps) { + mockPlatformInputProps = p + p.setInputRef({ + blur: () => {}, + clear: () => { + // the real hook drops suppressions from an effect after the text goes empty; a test + // can ask for that to happen synchronously inside clear() instead, which is the + // ordering onSubmit's snapshot has to survive + mockOnClear?.() + mockPlatformInputProps?.onChangeText('') + }, + focus: () => {}, + getSelection: () => undefined, + isFocused: () => false, + transformText: () => {}, + value: '', + }) + return null + }, +})) // useChatThreadRouteParams only honors params on the chat thread routes, so the mock needs a matching name jest.mock('@react-navigation/native', () => ({ useRoute: () => ({name: 'chatConversation', params: mockRouteParams}), @@ -86,6 +118,30 @@ const renderInput = (id = convID) => wrapper: wrapperFor(id), }) +function renderComposer(id = convID) { + return render(, {wrapper: wrapperFor(id)}) +} + +type InputHandles = { + input: ReturnType> + thread: ReturnType +} + +const InputProbe = (p: {onRender: (h: InputHandles) => void}) => { + p.onRender({input: useConversationInput(s => s), thread: useConversationThreadActions()}) + return null +} + +function renderComposerWithProbe(onRender: (h: InputHandles) => void, id = convID) { + return render( + <> + + + , + {wrapper: wrapperFor(id)} + ) +} + const renderInputWithThreadActions = (id = convID) => renderHook( () => ({ @@ -112,6 +168,8 @@ beforeEach(() => { }) afterEach(() => { + mockPlatformInputProps = undefined + mockOnClear = undefined cleanup() jest.restoreAllMocks() resetAllStores() @@ -396,6 +454,215 @@ test('giphy engine events and send path update the input owner', async () => { expect(result.current.input.unsentText).toBe('') }) +test('sendComposerText sends dismissed unfurl urls as unfurlSuppress', async () => { + const getLastPost = mockPostText() + useUnfurlPreviewState.getState().dispatch.dismiss(convID, ['http://a.com']) + const {result} = renderInput() + + act(() => { + result.current.dispatch.sendComposerText('hi http://a.com', {dismissed: ['http://a.com'], failed: []}) + }) + await flushPromises() + + expect(getLastPost()?.params.unfurlSuppress).toEqual(['http://a.com']) + expect(getSuppressedURLs(convID)).toEqual([]) +}) + +// a send that carries no snapshot is not the composer sending its own text -- a coinflip +// resend goes through the same action -- so it must not pick up the composer's dismissals, +// nor clear them when it lands +test('a send with no snapshot leaves the composer dismissals alone', async () => { + const getLastPost = mockPostText() + useUnfurlPreviewState.getState().dispatch.dismiss(convID, ['http://a.com']) + const {result} = renderInput() + + act(() => { + result.current.dispatch.sendComposerText('/flip 2') + }) + await flushPromises() + + expect(getLastPost()?.params.unfurlSuppress).toEqual([]) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) +}) + +test('onSubmit sends dismissed unfurl urls even though clearing the composer drops them', async () => { + jest.useFakeTimers() + try { + const getLastPost = mockPostText() + jest.spyOn(T.RPCChat, 'localUpdateTypingRpcPromise').mockResolvedValue(undefined) + jest.spyOn(T.RPCChat, 'localUpdateUnsentTextRpcPromise').mockResolvedValue(undefined) + jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([ + { + unfurl: {generic: {siteName: 'a', title: 'a', url: 'http://a.com'}, unfurlType: T.RPCChat.UnfurlType.generic}, + url: 'http://a.com', + } as T.RPCChat.UnfurlPreviewInfo, + ]) + renderComposer() + + const text = 'look at http://a.com' + act(() => { + mockPlatformInputProps?.onChangeText(text) + }) + // let the preview debounce fire so the hook holds a preview for this url + await act(async () => { + jest.advanceTimersByTime(600) + await flushPromises() + }) + + // the card's X + act(() => { + useUnfurlPreviewState.getState().dispatch.dismiss(convID, ['http://a.com']) + }) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) + + // more than the 200ms draft throttle since the last keystroke, so clearing the composer + // runs updateDraft's leading edge synchronously and the hook drops the dismissal + act(() => { + mockPlatformInputProps?.onSubmit(text) + }) + expect(getSuppressedURLs(convID)).toEqual([]) + + await act(async () => { + jest.advanceTimersByTime(1) + await flushPromises() + }) + + expect(getLastPost()?.params.body).toBe(text) + expect(getLastPost()?.params.unfurlSuppress).toEqual(['http://a.com']) + } finally { + jest.useRealTimers() + } +}) + +test('onSubmit snapshots the dismissals before the composer clears them', async () => { + jest.useFakeTimers() + try { + mockOnClear = () => useUnfurlPreviewState.getState().dispatch.keepOnly(convID, []) + const getLastPost = mockPostText() + jest.spyOn(T.RPCChat, 'localUpdateTypingRpcPromise').mockResolvedValue(undefined) + jest.spyOn(T.RPCChat, 'localUpdateUnsentTextRpcPromise').mockResolvedValue(undefined) + jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([]) + renderComposer() + act(() => { + useUnfurlPreviewState.getState().dispatch.dismiss(convID, ['http://a.com']) + }) + + act(() => { + mockPlatformInputProps?.onSubmit('look at http://a.com') + }) + // the clear has already emptied the store by now: only a snapshot taken ahead of it + // still has the dismissal to send + expect(getSuppressedURLs(convID)).toEqual([]) + + await act(async () => { + jest.advanceTimersByTime(1) + await flushPromises() + }) + + expect(getLastPost()?.params.unfurlSuppress).toEqual(['http://a.com']) + } finally { + jest.useRealTimers() + } +}) + +// the snapshot owns only what it took: a dismissal made while the send was in flight +// belongs to the next message, and clearing it would unfurl a card the user just declined +test('a landed send leaves a dismissal made while it was in flight alone', async () => { + mockPostText() + const {result} = renderInput() + + act(() => { + result.current.dispatch.sendComposerText('hi http://wsj.com', {dismissed: [], failed: ['http://wsj.com']}) + }) + useUnfurlPreviewState.getState().dispatch.dismiss(convID, ['http://wsj.com']) + await flushPromises() + + expect(getSuppressedURLs(convID)).toEqual(['http://wsj.com']) +}) + +// an edit posts as MessageType_EDIT, which the unfurler does not extract urls from, so +// there is nothing to preview and no scrape to pay for +test('no preview is fetched while editing', async () => { + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([]) + jest.spyOn(T.RPCChat, 'localUpdateTypingRpcPromise').mockResolvedValue(undefined) + jest.spyOn(T.RPCChat, 'localUpdateUnsentTextRpcPromise').mockResolvedValue(undefined) + jest.useFakeTimers() + try { + // the composer and the handles that drive it have to share one provider, or the editing + // state never reaches the composer under test + let handles: InputHandles | undefined + renderComposerWithProbe(h => (handles = h)) + act(() => { + handles?.thread.addMessages([makeTextMessage({text: 'look at http://a.com'})], {markAsRead: false}) + }) + act(() => { + mockPlatformInputProps?.onChangeText('look at http://a.com') + }) + act(() => { + handles?.input.dispatch.setEditing('last') + }) + await act(async () => { + jest.advanceTimersByTime(1000) + await flushPromises() + }) + expect(spy).not.toHaveBeenCalled() + } finally { + jest.useRealTimers() + } +}) + +test('a canceled stellar send restores the dismissed unfurl urls for the resend', async () => { + // the composer clears its dismissals before the send resolves, so a cancel has to put + // the snapshot back or the restored text re-unfurls what the user dismissed + jest.spyOn(T.RPCChat, 'localPostTextNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatStellarDone']?.({canceled: true}) + await Promise.resolve() + return {outboxID: makeRpcOutboxID('posted-outbox')} + }) + const {result} = renderInput() + + act(() => { + result.current.dispatch.sendComposerText('hi http://a.com', {dismissed: ['http://a.com'], failed: []}) + }) + await flushPromises() + + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) +}) + +test('a canceled stellar send leaves a failed preview unrecorded as a dismissal', async () => { + // a failure is re-derived by the next fetch, a dismissal never is, so restoring one as + // the other would keep the url suppressed even after it starts scraping again + jest.spyOn(T.RPCChat, 'localPostTextNonblockRpcListener').mockImplementation(async p => { + p.incomingCallMap['chat.1.chatUi.chatStellarDone']?.({canceled: true}) + await Promise.resolve() + return {outboxID: makeRpcOutboxID('posted-outbox')} + }) + const {result} = renderInput() + + act(() => { + result.current.dispatch.sendComposerText('hi http://wsj.com', {dismissed: [], failed: ['http://wsj.com']}) + }) + await flushPromises() + + expect(useUnfurlPreviewState.getState().dismissed.get(convID)).toBeUndefined() + expect(getSuppressedURLs(convID)).toEqual([]) +}) + +test('a send suppresses what failed to preview as well as what was dismissed', async () => { + const getLastPost = mockPostText() + const {result} = renderInput() + + act(() => { + result.current.dispatch.sendComposerText('hi http://a.com http://wsj.com', { + dismissed: ['http://a.com'], + failed: ['http://wsj.com'], + }) + }) + await flushPromises() + + expect(getLastPost()?.params.unfurlSuppress).toEqual(['http://a.com', 'http://wsj.com']) +}) + test('toggleGiphyPrefill toggles the slash command text', () => { const {result} = renderInput() diff --git a/shared/chat/conversation/input-area/input-state.tsx b/shared/chat/conversation/input-area/input-state.tsx index c58d2da73965..b2b7178c0c38 100644 --- a/shared/chat/conversation/input-area/input-state.tsx +++ b/shared/chat/conversation/input-area/input-state.tsx @@ -7,6 +7,7 @@ import {useCurrentUserState} from '@/stores/current-user' import {useEngineActionListener} from '@/engine/action-listener' import {useConversationThreadStore} from '../thread-context' import {useConversationSendActions} from '../send-actions' +import type {SuppressSnapshot} from '../unfurl-preview-state' type ConversationInputStore = T.Immutable<{ commandMarkdown?: T.RPCChat.UICommandMarkdown @@ -22,7 +23,7 @@ type ConversationInputStore = T.Immutable<{ type ConversationInputDispatch = { injectIntoInput: (text?: string, focus?: boolean) => void resetState: () => void - sendComposerText: (text: string) => void + sendComposerText: (text: string, unfurlSuppress?: SuppressSnapshot) => void sendGiphyResult: (result: T.RPCChat.GiphySearchResult) => void setCommandMarkdown: (md?: T.RPCChat.UICommandMarkdown) => void setCommandStatusInfo: (info?: T.Chat.CommandStatusInfo) => void @@ -174,11 +175,12 @@ export const ConversationInputProvider = (p: React.PropsWithChildren<{id: T.Chat }) } }) - const sendComposerText = React.useEffectEvent((text: string) => { + const sendComposerText = React.useEffectEvent((text: string, unfurlSuppress?: SuppressSnapshot) => { sendMessage(text, { editingOrdinal: state.editing, onRestoreText: injectIntoInput, replyToOrdinal: state.replyTo, + unfurlSuppress, }) dispatchState({type: 'afterSend'}) }) diff --git a/shared/chat/conversation/input-area/normal/index.tsx b/shared/chat/conversation/input-area/normal/index.tsx index 4201177d775a..58736824dd4c 100644 --- a/shared/chat/conversation/input-area/normal/index.tsx +++ b/shared/chat/conversation/input-area/normal/index.tsx @@ -7,6 +7,7 @@ import Giphy from '../../giphy' import * as InputState from '../input-state' import PlatformInput from './input' import ReplyPreview from '../../reply-preview' +import UnfurlPreview from '../unfurl-preview' import * as T from '@/constants/types' import {indefiniteArticle} from '@/util/string' import {infoPanelWidthTablet} from '../../info-panel/common' @@ -26,6 +27,7 @@ import {useConversationParticipantsSelector} from '../../data-hooks' import {useCurrentUserState} from '@/stores/current-user' import {useRoute} from '@react-navigation/native' import {metasReceived, unboxRows, useInboxMetadataState} from '@/chat/inbox/metadata' +import {takeSuppressSnapshot} from '@/chat/conversation/unfurl-preview-state' const useHintText = (p: { isExploding: boolean @@ -207,9 +209,12 @@ const ConnectedPlatformInput = function ConnectedPlatformInput() { // A timeout rather than requestAnimationFrame: this callback owns the only copy of the text, and // frames stop in a hidden or backgrounded window, which would drop the message with the composer // already emptied. + // Before the clear, which runs onChangeText('') synchronously and drops every dismissal. + // Urls whose preview has not landed yet are not in here and so are not suppressed. + const unfurlSuppress = takeSuppressSnapshot(conversationIDKey) injectText('', true) setTimeout(() => { - sendComposerText(text) + sendComposerText(text, unfurlSuppress) if (hasCenter) { toggleThreadSearch(true) jumpToRecent() @@ -228,6 +233,10 @@ const ConnectedPlatformInput = function ConnectedPlatformInput() { } const sendTyping = C.useThrottledCallback(sendTypingRaw, 1000) + // Low-frequency copy of the composer text for the unfurl preview, set from the already + // throttled draft-save path rather than from onChangeText, so the composer does not + // re-render on every keystroke. The preview debounces another 500ms downstream anyway. + const [previewText, setPreviewText] = React.useState('') const updateDraftRaw = (text: string) => { // Immediately update local meta.draft so switching back to this thread // before the async unbox completes won't re-inject the old stale draft. @@ -236,6 +245,7 @@ const ConnectedPlatformInput = function ConnectedPlatformInput() { if (currentMeta) { metasReceived([{...currentMeta, draft: text}], undefined, {force: true}) } + setPreviewText(text) const f = async () => { await T.RPCChat.localUpdateUnsentTextRpcPromise({ conversationID: convoID, @@ -309,7 +319,7 @@ const ConnectedPlatformInput = function ConnectedPlatformInput() { setInputRef(inputRef.current) }, [setInputRef]) - return ( + const input = ( ) + + // nothing while editing: an edit posts as MessageType_EDIT, which the unfurler does not + // extract urls from at all, so a card would promise an unfurl the edit cannot produce + const preview = isEditing ? null : ( + + ) + + if (isMobile) { + // in flow above the composer; it rides KeyboardStickyView with the input + return ( + <> + {preview} + {input} + + ) + } + + // the preview floats out of this box, so it needs a positioned ancestor + return ( + + {preview} + {input} + + ) } const useStyles = Kb.Styles.createStyleHook(() => { diff --git a/shared/chat/conversation/input-area/unfurl-preview.test.tsx b/shared/chat/conversation/input-area/unfurl-preview.test.tsx new file mode 100644 index 000000000000..b52d0b7712ae --- /dev/null +++ b/shared/chat/conversation/input-area/unfurl-preview.test.tsx @@ -0,0 +1,146 @@ +/** @jest-environment jsdom */ +/// +import * as T from '@/constants/types' +import {fireEvent, render} from '@testing-library/react' +import UnfurlPreview from './unfurl-preview' +import {ThreadRefsContext} from '@/chat/conversation/normal/context' + +const mockDismiss = jest.fn() +let mockPreviews: ReadonlyArray = [] + +jest.mock('@/chat/conversation/unfurl-preview-state', () => ({ + useUnfurlPreviews: () => ({dismiss: mockDismiss, previews: mockPreviews}), +})) + +const convID = T.Chat.conversationIDToKey(new Uint8Array([1, 2, 3, 4])) + +const nonGenericInfo: T.RPCChat.UnfurlPreviewInfo = { + unfurl: {unfurlType: T.RPCChat.UnfurlType.youtube, youtube: {}}, + url: 'http://youtube.com/watch', +} as T.RPCChat.UnfurlPreviewInfo + +const genericInfo2: T.RPCChat.UnfurlPreviewInfo = { + unfurl: {generic: {siteName: 'b', title: 'Bravo', url: 'http://b.com'}, unfurlType: T.RPCChat.UnfurlType.generic}, + url: 'http://b.com', +} as T.RPCChat.UnfurlPreviewInfo + +const genericInfo: T.RPCChat.UnfurlPreviewInfo = { + unfurl: {generic: {siteName: 'a', title: 'Alpha', url: 'http://a.com'}, unfurlType: T.RPCChat.UnfurlType.generic}, + url: 'http://a.com', +} as T.RPCChat.UnfurlPreviewInfo + +const mapInfo: T.RPCChat.UnfurlPreviewInfo = { + unfurl: { + generic: {mapInfo: {isLiveLocationDone: true}, siteName: 'Google Maps', title: 'here', url: 'http://map.com'}, + unfurlType: T.RPCChat.UnfurlType.generic, + }, + url: 'http://map.com', +} as T.RPCChat.UnfurlPreviewInfo + +// these render the desktop tree only: Kb.Box2 renders a react-native Pressable when +// isMobile is set, which produces no DOM under jsdom, so flipping that global here would +// assert nothing. the mobile layout is verified on a device. +describe('UnfurlPreview', () => { + afterEach(() => { + mockPreviews = [] + }) + + it('renders nothing when every preview is non-generic', () => { + mockPreviews = [nonGenericInfo] + const {container} = render() + expect(container.firstChild).toBeNull() + }) + + it('renders a card for a generic preview', () => { + mockPreviews = [genericInfo] + const {container} = render() + expect(container.firstChild).not.toBeNull() + }) + + it('renders nothing for a map unfurl', () => { + mockPreviews = [mapInfo] + const {container} = render() + expect(container.firstChild).toBeNull() + }) + + it('shows no pager for a single preview', () => { + mockPreviews = [genericInfo] + const {queryByText} = render() + expect(queryByText('1/1')).toBeNull() + }) + + it('pages between previews and disables the arrows at each end', () => { + mockPreviews = [genericInfo, genericInfo2] + const {getByText, container} = render( + + ) + expect(getByText('1/2')).toBeTruthy() + // the card shown is the first one + expect(container.textContent).toContain('Alpha') + + const left = container.querySelector('.icon-gen-iconfont-arrow-left') as Element + const right = container.querySelector('.icon-gen-iconfont-arrow-right') as Element + + // at the start the left arrow does nothing + fireEvent.click(left) + expect(getByText('1/2')).toBeTruthy() + + fireEvent.click(right) + expect(getByText('2/2')).toBeTruthy() + expect(container.textContent).toContain('Bravo') + + // at the end the right arrow does nothing + fireEvent.click(right) + expect(getByText('2/2')).toBeTruthy() + }) + + it('re-clamps the index when the shown card is dismissed away', () => { + mockPreviews = [genericInfo, genericInfo2] + const {getByText, container, rerender} = render( + + ) + fireEvent.click(container.querySelector('.icon-gen-iconfont-arrow-right') as Element) + expect(getByText('2/2')).toBeTruthy() + + // the second preview goes away; the index must fall back rather than blank the panel + mockPreviews = [genericInfo] + rerender() + expect(container.textContent).toContain('Alpha') + }) + + it('dismisses the shown card by its url when the close icon is clicked', () => { + mockPreviews = [genericInfo, genericInfo2] + const {container} = render( + + ) + fireEvent.click(container.querySelector('.icon-gen-iconfont-close') as Element) + expect(mockDismiss).toHaveBeenCalledWith('http://a.com') + + // and after paging it dismisses the one actually on screen, not the first + mockDismiss.mockClear() + fireEvent.click(container.querySelector('.icon-gen-iconfont-arrow-right') as Element) + fireEvent.click(container.querySelector('.icon-gen-iconfont-close') as Element) + expect(mockDismiss).toHaveBeenCalledWith('http://b.com') + }) + + it('puts focus back in the composer after a dismiss', () => { + mockPreviews = [genericInfo] + const focusInput = jest.fn() + const refs = { + focusInput, + scrollDown: () => {}, + scrollToBottom: () => {}, + scrollUp: () => {}, + setInputRef: () => {}, + setScrollRef: () => {}, + } + const {container} = render( + + + + ) + fireEvent.click(container.querySelector('.icon-gen-iconfont-close') as Element) + expect(focusInput).toHaveBeenCalled() + }) + +}) diff --git a/shared/chat/conversation/input-area/unfurl-preview.tsx b/shared/chat/conversation/input-area/unfurl-preview.tsx new file mode 100644 index 000000000000..49e18dc36466 --- /dev/null +++ b/shared/chat/conversation/input-area/unfurl-preview.tsx @@ -0,0 +1,139 @@ +import * as Kb from '@/common-adapters' +import * as React from 'react' +import * as T from '@/constants/types' +import UnfurlGenericView from '@/chat/conversation/messages/text/unfurl/unfurl-list/generic-view' +import {useUnfurlPreviews} from '@/chat/conversation/unfurl-preview-state' +import {ThreadRefsContext} from '@/chat/conversation/normal/context' + +type Props = { + conversationIDKey: T.Chat.ConversationIDKey + text: string +} + +const UnfurlPreview = (p: Props) => { + const {conversationIDKey, text} = p + const styles = useStyles() + const theme = Kb.Styles.useTheme() + const {dismiss, previews} = useUnfurlPreviews(conversationIDKey, text) + const {focusInput} = React.useContext(ThreadRefsContext) + const [index, setIndex] = React.useState(0) + const genericPreviews = previews.flatMap(preview => { + const {unfurl} = preview + if (unfurl.unfurlType !== T.RPCChat.UnfurlType.generic || unfurl.generic.mapInfo) { + // a map unfurl is a generic unfurl with mapInfo set, and the message card refuses + // to render those, so the preview must not show one either + return [] + } + return [{generic: unfurl.generic, preview}] + }) + // dismissing the last card, or the text losing a link, shrinks the list under us + const clamped = Math.min(index, Math.max(genericPreviews.length - 1, 0)) + const shown = genericPreviews[clamped] + if (!shown) { + return null + } + const {generic, preview} = shown + const onPrevious = () => { + setIndex(clamped - 1) + } + const onNext = () => { + setIndex(clamped + 1) + } + const atStart = clamped === 0 + const atEnd = clamped === genericPreviews.length - 1 + const pager = + genericPreviews.length > 1 ? ( + + + {`${clamped + 1}/${genericPreviews.length}`} + + + ) : null + const card = ( + { + dismiss(preview.url) + // the X takes focus on the way out, and the composer is where the user was + focusInput() + }} + publishTime={generic.publishTime ?? undefined} + siteName={generic.siteName} + title={generic.title} + url={generic.url} + /> + ) + return ( + + {pager} + {/* the card area is a fixed size, so paging between cards never resizes the panel. + only this part scrolls; the pager above it stays put */} + {isMobile ? ( + // native clips at a fixed height rather than scrolling, so it needs a real scroller + {card} + ) : ( + + {card} + + )} + + ) +} + +const useStyles = Kb.Styles.createStyleHook( + theme => + ({ + cardArea: Kb.Styles.platformStyles({ + common: {height: 200}, + // per axis rather than the shorthand: `overflow: hidden` would also set the y axis + // and beat the scroll depending on emission order + isElectron: {overflowX: 'hidden', overflowY: 'auto', width: 420}, + isMobile: {flexGrow: 0, flexShrink: 0}, + }), + container: Kb.Styles.platformStyles({ + common: { + backgroundColor: theme.white, + borderColor: theme.black_10, + borderRadius: Kb.Styles.borderRadius, + borderWidth: 1, + padding: Kb.Styles.globalMargins.tiny, + }, + isElectron: { + ...Kb.Styles.desktopStyles.boxShadow, + // floats over the thread instead of taking flow space, so showing a preview + // never shifts the message list + borderStyle: 'solid', + bottom: '100%', + left: Kb.Styles.globalMargins.small, + marginBottom: Kb.Styles.globalMargins.xtiny, + position: 'absolute', + }, + isMobile: { + // in flow above the composer, the way the reply preview already sits + alignSelf: 'stretch', + marginBottom: Kb.Styles.globalMargins.xtiny, + marginLeft: Kb.Styles.globalMargins.tiny, + marginRight: Kb.Styles.globalMargins.tiny, + }, + }), + pager: Kb.Styles.platformStyles({ + common: {alignItems: 'center'}, + }), + }) as const +) + +export default UnfurlPreview diff --git a/shared/chat/conversation/messages/text/unfurl/unfurl-list/generic-view.tsx b/shared/chat/conversation/messages/text/unfurl/unfurl-list/generic-view.tsx new file mode 100644 index 000000000000..5f5976f9cd8f --- /dev/null +++ b/shared/chat/conversation/messages/text/unfurl/unfurl-list/generic-view.tsx @@ -0,0 +1,167 @@ +import * as Kb from '@/common-adapters/index' +import type * as T from '@/constants/types' +import UnfurlImage from './image' +import {formatTimeForMessages} from '@/util/timestamp' + +export type UnfurlGenericViewProps = { + description?: string + favicon?: T.RPCChat.UnfurlImageDisplay + isCollapsed?: boolean + media?: T.RPCChat.UnfurlImageDisplay + onClose?: () => void + onToggleCollapse?: () => void + publishTime?: number + siteName: string + title: string + url: string +} + +export default function UnfurlGenericView(p: UnfurlGenericViewProps) { + const {description, favicon, isCollapsed, media, onClose, onToggleCollapse, publishTime, siteName, title, url} = p + const styles = useStyles() + const theme = Kb.Styles.useTheme() + const titleUrlProps = Kb.useClickURL(url) + const {height, width, isVideo, url: mediaUrl} = media || {height: 0, isVideo: false, url: '', width: 0} + const showImageOnSide = + !isMobile && height >= width && !isVideo && (title.length > 0 || !!description) + const imageLocation = isCollapsed ? 'collapsed' : showImageOnSide ? 'side' : width > 0 && height > 0 ? 'bottom' : 'none' + + const publisher = ( + + {favicon?.url ? : null} + + + {siteName} + {publishTime ? ( + • Published {formatTimeForMessages(publishTime * 1000)} + ) : null} + + + {onClose ? ( + + ) : null} + + ) + + const snippet = description ? ( + + {description} + {(imageLocation === 'collapsed' || imageLocation === 'bottom') && ( + <> + {' '} + + + )} + + ) : null + + const bottomImage = + imageLocation === 'bottom' ? ( + + + + ) : null + + const rightImage = + imageLocation === 'side' && mediaUrl ? ( + + + + ) : null + + return ( + + {!isMobile && } + + {publisher} + + {title} + + {snippet} + {bottomImage} + + {rightImage} + + ) +} + +const useStyles = Kb.Styles.createStyleHook( + theme => + ({ + bottomImage: Kb.Styles.platformStyles({ + common: {marginTop: Kb.Styles.globalMargins.xtiny}, + isMobile: {alignSelf: 'center'}, + }), + closeBox: Kb.Styles.platformStyles({ + isElectron: { + alignSelf: 'flex-start', + marginLeft: 'auto', + }, + }), + collapseBox: Kb.Styles.platformStyles({ + isElectron: {display: 'inline'}, + }), + container: Kb.Styles.platformStyles({ + isElectron: {maxWidth: 500}, + isTablet: {maxWidth: 500}, + }), + + favicon: Kb.Styles.platformStyles({ + common: { + borderRadius: Kb.Styles.borderRadius, + ...Kb.Styles.size(16), + }, + }), + innerContainer: Kb.Styles.platformStyles({ + common: { + minWidth: 150, + }, + isMobile: { + borderColor: theme.grey, + borderRadius: Kb.Styles.borderRadius, + borderWidth: 1, + padding: Kb.Styles.globalMargins.xtiny, + }, + }), + quoteContainer: Kb.Styles.platformStyles({ + common: { + backgroundColor: theme.grey, + paddingLeft: Kb.Styles.globalMargins.xtiny, + }, + }), + sideImage: Kb.Styles.platformStyles({ + isElectron: { + ...Kb.Styles.size(80), + }, + }), + siteNameContainer: Kb.Styles.platformStyles({ + isElectron: {minHeight: 16}, + isMobile: {minHeight: 21}, + }), + url: { + ...Kb.Styles.globalStyles.fontSemibold, + }, + }) as const +) diff --git a/shared/chat/conversation/messages/text/unfurl/unfurl-list/generic.tsx b/shared/chat/conversation/messages/text/unfurl/unfurl-list/generic.tsx index 2878b06d2edf..75b8a4d304eb 100644 --- a/shared/chat/conversation/messages/text/unfurl/unfurl-list/generic.tsx +++ b/shared/chat/conversation/messages/text/unfurl/unfurl-list/generic.tsx @@ -1,18 +1,12 @@ -import * as Kb from '@/common-adapters/index' import * as T from '@/constants/types' -import UnfurlImage from './image' -import {formatTimeForMessages} from '@/util/timestamp' +import UnfurlGenericView from './generic-view' import {useActions} from './use-state' function UnfurlGeneric(p: { - author: string - conversationIDKey: T.Chat.ConversationIDKey ordinal: T.Chat.Ordinal unfurlInfo: T.RPCChat.UIMessageUnfurlInfo youAreAuthor: boolean }) { - const styles = useStyles() - const theme = Kb.Styles.useTheme() const {ordinal, unfurlInfo, youAreAuthor} = p const {isCollapsed, unfurl, unfurlMessageID} = unfurlInfo const {onClose, onToggleCollapse} = useActions( @@ -21,154 +15,25 @@ function UnfurlGeneric(p: { ordinal ) const generic = unfurl.unfurlType === T.RPCChat.UnfurlType.generic ? unfurl.generic : undefined - const titleUrlProps = Kb.useClickURL(generic?.mapInfo ? '' : (generic?.url ?? '')) if (!generic || generic.mapInfo) { return null } const {description, publishTime, favicon, media, siteName, title, url} = generic - const {height, width, isVideo, url: mediaUrl} = media || {height: 0, isVideo: false, url: '', width: 0} - const showImageOnSide = - !isMobile && height >= width && !isVideo && (title.length > 0 || !!description) - const imageLocation = isCollapsed ? 'collapsed' : showImageOnSide ? 'side' : width > 0 && height > 0 ? 'bottom' : 'none' - - const publisher = ( - - {favicon?.url ? : null} - - - {siteName} - {publishTime ? ( - • Published {formatTimeForMessages(publishTime * 1000)} - ) : null} - - - {onClose ? ( - - ) : null} - - ) - - const snippet = description ? ( - - {description} - {(imageLocation === 'collapsed' || imageLocation === 'bottom') && ( - <> - {' '} - - - )} - - ) : null - - const bottomImage = - imageLocation === 'bottom' ? ( - - - - ) : null - - const rightImage = - imageLocation === 'side' && mediaUrl ? ( - - - - ) : null return ( - - {!isMobile && } - - {publisher} - - {title} - - {snippet} - {bottomImage} - - {rightImage} - + ) } -const useStyles = Kb.Styles.createStyleHook( - theme => - ({ - bottomImage: Kb.Styles.platformStyles({ - common: {marginTop: Kb.Styles.globalMargins.xtiny}, - isMobile: {alignSelf: 'center'}, - }), - closeBox: Kb.Styles.platformStyles({ - isElectron: { - alignSelf: 'flex-start', - marginLeft: 'auto', - }, - }), - collapseBox: Kb.Styles.platformStyles({ - isElectron: {display: 'inline'}, - }), - container: Kb.Styles.platformStyles({ - isElectron: {maxWidth: 500}, - isTablet: {maxWidth: 500}, - }), - - favicon: Kb.Styles.platformStyles({ - common: { - borderRadius: Kb.Styles.borderRadius, - ...Kb.Styles.size(16), - }, - }), - innerContainer: Kb.Styles.platformStyles({ - common: { - minWidth: 150, - }, - isMobile: { - borderColor: theme.grey, - borderRadius: Kb.Styles.borderRadius, - borderWidth: 1, - padding: Kb.Styles.globalMargins.xtiny, - }, - }), - quoteContainer: Kb.Styles.platformStyles({ - common: { - backgroundColor: theme.grey, - paddingLeft: Kb.Styles.globalMargins.xtiny, - }, - }), - sideImage: Kb.Styles.platformStyles({ - isElectron: { - ...Kb.Styles.size(80), - }, - }), - siteNameContainer: Kb.Styles.platformStyles({ - isElectron: {minHeight: 16}, - isMobile: {minHeight: 21}, - }), - url: { - ...Kb.Styles.globalStyles.fontSemibold, - }, - }) as const -) - export default UnfurlGeneric diff --git a/shared/chat/conversation/send-actions.tsx b/shared/chat/conversation/send-actions.tsx index 75ab4c1302ac..372232587a80 100644 --- a/shared/chat/conversation/send-actions.tsx +++ b/shared/chat/conversation/send-actions.tsx @@ -4,6 +4,7 @@ import logger from '@/logger' import {RPCError} from '@/util/errors' import {ignorePromise} from '@/constants/utils' import {getClientPrevFromThread} from './attachment-actions' +import {removeDismissals, restoreDismissals, suppressedURLsOf, type SuppressSnapshot} from './unfurl-preview-state' import {useInboxMetadataState} from '../inbox/metadata-store' import { useConversationThreadActions, @@ -16,15 +17,20 @@ type SendTextParams = { conversationIDKey: T.Chat.ConversationIDKey ephemeralLifetime: number onRestoreText?: (text: string) => void + onSent?: () => void replyTo?: T.Chat.MessageID text: string tlfName: string + unfurlSuppress?: ReadonlyArray waitingKey?: string } const sendTextMessageStoreless = (p: SendTextParams) => { const f = async () => { const ephemeralData = p.ephemeralLifetime !== 0 ? {ephemeralLifetime: p.ephemeralLifetime} : {} + // a canceled stellar confirm resolves the rpc normally but posts nothing, so it is + // not a send and must not be treated as one + const sendState = {stellarCanceled: false} try { await T.RPCChat.localPostTextNonblockRpcListener({ customResponseIncomingCallMap: { @@ -38,6 +44,7 @@ const sendTextMessageStoreless = (p: SendTextParams) => { incomingCallMap: { 'chat.1.chatUi.chatStellarDone': ({canceled}) => { if (canceled) { + sendState.stellarCanceled = true p.onRestoreText?.(p.text) } }, @@ -53,9 +60,13 @@ const sendTextMessageStoreless = (p: SendTextParams) => { replyTo: p.replyTo, tlfName: p.tlfName, tlfPublic: false, + unfurlSuppress: p.unfurlSuppress ? [...p.unfurlSuppress] : [], }, waitingKey: p.waitingKey, }) + if (!sendState.stellarCanceled) { + p.onSent?.() + } logger.info('success') } catch { logger.info('error') @@ -127,23 +138,44 @@ export const useConversationSendActions = () => { editingOrdinal?: T.Chat.Ordinal onRestoreText?: (text: string) => void replyToOrdinal?: T.Chat.Ordinal + unfurlSuppress?: SuppressSnapshot } ) => { const editOrdinal = context?.editingOrdinal if (editOrdinal) { + // unfurlSuppress is dropped here on purpose: postEditNonblock has no such param, so + // dismissing a preview card while editing cannot reach the service. carrying it + // would need the protocol change, not just plumbing on this side. editMessage(editOrdinal, text) return } const replyToOrdinal = context?.replyToOrdinal const replyTo = threadStore.getState().messageMap.get(replyToOrdinal ?? T.Chat.numberToOrdinal(0))?.id + // only what the caller snapshotted. a send that carries no snapshot is not the composer + // sending its own text (a coinflip resend, say), and the composer's dismissals have + // nothing to do with it + const snapshot = context?.unfurlSuppress ?? {dismissed: [], failed: []} + const unfurlSuppress = suppressedURLsOf(snapshot) + const onRestoreText = context?.onRestoreText sendTextMessageStoreless({ clientPrev: getClientPrev(), conversationIDKey, ephemeralLifetime: threadStore.getState().explodingMode, - onRestoreText: context?.onRestoreText, + onRestoreText: onRestoreText + ? (restored: string) => { + restoreDismissals(conversationIDKey, snapshot.dismissed) + onRestoreText(restored) + } + : undefined, + onSent: () => { + // the dismissals only: a failure is never in `dismissed`, and the url may have been + // dismissed afresh while this send was in flight + removeDismissals(conversationIDKey, snapshot.dismissed) + }, replyTo, text, tlfName: getTlfName(), + unfurlSuppress, }) } diff --git a/shared/chat/conversation/unfurl-preview-state.test.tsx b/shared/chat/conversation/unfurl-preview-state.test.tsx new file mode 100644 index 000000000000..7047a1d6611f --- /dev/null +++ b/shared/chat/conversation/unfurl-preview-state.test.tsx @@ -0,0 +1,307 @@ +/** @jest-environment jsdom */ +/// +import * as T from '@/constants/types' +import {act, render, waitFor} from '@testing-library/react' +import {useUnfurlPreviews, suppressedURLsOf, takeSuppressSnapshot, useUnfurlPreviewState} from './unfurl-preview-state' + +const getSuppressedURLs = (c: T.Chat.ConversationIDKey) => suppressedURLsOf(takeSuppressSnapshot(c)) + +// stringToConversationIDKey('conv1') is not valid hex and would throw inside +// T.Chat.keyToConversationID (used to build the RPC's convID param), so build +// the fixture the way input-state.test.tsx does: round-trip through bytes. +const convID = T.Chat.conversationIDToKey(new Uint8Array([1, 2, 3, 4])) +const info = (url: string): T.RPCChat.UnfurlPreviewInfo => + ({unfurl: {generic: {title: url, url}, unfurlType: T.RPCChat.UnfurlType.generic}, url}) as T.RPCChat.UnfurlPreviewInfo + +// a url the service could not scrape: reported so it can be suppressed, no unfurl on it +const failedInfo = (url: string): T.RPCChat.UnfurlPreviewInfo => ({url}) as T.RPCChat.UnfurlPreviewInfo + +const Harness = (p: { + text: string + id?: T.Chat.ConversationIDKey + onRender: (r: ReturnType) => void +}) => { + const r = useUnfurlPreviews(p.id ?? convID, p.text) + p.onRender(r) + return null +} + +describe('unfurl previews', () => { + beforeEach(() => { + jest.useFakeTimers() + useUnfurlPreviewState.getState().dispatch.resetState() + }) + afterEach(() => { + jest.useRealTimers() + jest.restoreAllMocks() + }) + + it('does not call the rpc for text with no link', () => { + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([]) + render( {}} />) + act(() => { + jest.advanceTimersByTime(1000) + }) + expect(spy).not.toHaveBeenCalled() + }) + + it('debounces and returns previews', async () => { + const spy = jest + .spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + .mockResolvedValue([info('http://a.com')]) + let last: ReturnType | undefined + render( (last = r)} />) + expect(spy).not.toHaveBeenCalled() + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + expect(spy).toHaveBeenCalledTimes(1) + }) + + it('drops a stale response', async () => { + let resolveFirst: ((v: Array) => void) | undefined + jest + .spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + .mockImplementationOnce( + async () => new Promise>(resolve => (resolveFirst = resolve)) + ) + .mockResolvedValueOnce([info('http://b.com')]) + let last: ReturnType | undefined + const {rerender} = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + rerender( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews[0]?.url).toBe('http://b.com')) + act(() => resolveFirst?.([info('http://a.com')])) + expect(last?.previews[0]?.url).toBe('http://b.com') + }) + + it('dismiss hides the card and records the url for send', async () => { + jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([info('http://a.com')]) + let last: ReturnType | undefined + render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + act(() => last?.dismiss('http://a.com')) + await waitFor(() => expect(last?.previews.length).toBe(0)) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) + }) + + it('suppresses a url the service could not preview, and shows no card for it', async () => { + jest + .spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + .mockResolvedValue([failedInfo('http://wsj.com'), info('http://a.com')]) + let last: ReturnType | undefined + render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.map(p => p.url)).toEqual(['http://a.com'])) + // the send path would unfurl wsj minutes later otherwise, with no card to decline + expect(getSuppressedURLs(convID)).toEqual(['http://wsj.com']) + }) + + it('offers the card again once a url that failed starts previewing', async () => { + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + spy.mockResolvedValueOnce([failedInfo('http://a.com')]) + spy.mockResolvedValueOnce([info('http://a.com')]) + let last: ReturnType | undefined + const {rerender} = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(getSuppressedURLs(convID)).toEqual(['http://a.com'])) + rerender( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + expect(getSuppressedURLs(convID)).toEqual([]) + }) + + it('keeps a dismissal that the next fetch still returns', async () => { + // keepOnly prunes what the fetch no longer mentions; a url still in the result and + // still dismissed has to survive, or the card the user declined comes back + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + spy.mockResolvedValue([info('http://a.com')]) + let last: ReturnType | undefined + const {rerender} = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + act(() => last?.dismiss('http://a.com')) + rerender( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(spy).toHaveBeenCalledTimes(2)) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) + expect(last?.previews.length).toBe(0) + }) + + it('drops the card for a url the user has typed on past', async () => { + // the old url is a prefix of the new one, so a substring test would keep the stale card + // showing and let its X suppress a link the message does not contain + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + spy.mockResolvedValueOnce([info('http://a.com')]) + spy.mockResolvedValueOnce([info('http://a.com/foo')]) + let last: ReturnType | undefined + const {rerender} = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + rerender( (last = r)} />) + await waitFor(() => expect(last?.previews.length).toBe(0)) + // and the card comes back once the fetch for the longer url lands + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.map(p => p.url)).toEqual(['http://a.com/foo'])) + }) + + it('keeps showing a card when the url is followed by punctuation', async () => { + jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([info('http://a.com')]) + let last: ReturnType | undefined + const {rerender} = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + rerender( (last = r)} />) + expect(last?.previews.length).toBe(1) + }) + + it('sends a url that was both dismissed and unpreviewable only once', () => { + useUnfurlPreviewState.getState().dispatch.dismiss(convID, ['http://a.com']) + useUnfurlPreviewState.getState().dispatch.setFailed(convID, ['http://a.com', 'http://wsj.com']) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com', 'http://wsj.com']) + }) + + it('replaces the failed set wholesale rather than accumulating', async () => { + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + spy.mockResolvedValueOnce([failedInfo('http://a.com'), failedInfo('http://b.com')]) + spy.mockResolvedValueOnce([info('http://a.com'), failedInfo('http://b.com')]) + const {rerender} = render( {}} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(getSuppressedURLs(convID)).toEqual(['http://a.com', 'http://b.com'])) + rerender( {}} />) + act(() => { + jest.advanceTimersByTime(500) + }) + // a recovered to a card while b is still failing: keeping a suppressed would hide the + // card the composer is now showing + await waitFor(() => expect(getSuppressedURLs(convID)).toEqual(['http://b.com'])) + }) + + it('forgets a failure once the url leaves the text', async () => { + jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([failedInfo('http://a.com')]) + const {rerender} = render( {}} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(getSuppressedURLs(convID)).toEqual(['http://a.com'])) + rerender( {}} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(getSuppressedURLs(convID)).toEqual([])) + }) + + it('drops a response left in flight by a mount that has gone away', async () => { + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + let resolveFirst: (infos: ReadonlyArray) => void = () => {} + spy.mockImplementationOnce( + async () => new Promise>(resolve => (resolveFirst = resolve)) + ) + spy.mockResolvedValueOnce([info('http://a.com')]) + // the conversation the user leaves, with a scrape still running + const first = render( {}} />) + act(() => { + jest.advanceTimersByTime(500) + }) + first.unmount() + // and the one they come back to, which finishes its own fetch first + let last: ReturnType | undefined + render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + await act(async () => { + resolveFirst([failedInfo('http://a.com')]) + await Promise.resolve() + }) + expect(getSuppressedURLs(convID)).toEqual([]) + expect(last?.previews.length).toBe(1) + }) + + it('forgets a dismissal once the url leaves the text', async () => { + jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([]) + let last: ReturnType | undefined + const {rerender} = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + act(() => last?.dismiss('http://a.com')) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) + rerender( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(getSuppressedURLs(convID)).toEqual([])) + }) + + it('drops a card once its url leaves the composer, even if the next fetch fails', async () => { + const spy = jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise') + spy.mockResolvedValueOnce([info('http://a.com')]) + spy.mockRejectedValueOnce(new Error('scrape failed')) + let last: ReturnType | undefined + const {rerender} = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews[0]?.url).toBe('http://a.com')) + + // the user replaces the link; the fetch for the new one fails, so nothing ever + // overwrites the previous result. the old card must not stay on screen, or its X would + // suppress a url that is no longer in the message while the new one goes out unfurled + rerender( (last = r)} />) + expect(last?.previews).toEqual([]) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews).toEqual([])) + }) + + it('keeps dismissals when the conversation is left and returned to', async () => { + jest.spyOn(T.RPCChat, 'localUnfurlPreviewLocalRpcPromise').mockResolvedValue([info('http://a.com')]) + let last: ReturnType | undefined + const first = render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + await waitFor(() => expect(last?.previews.length).toBe(1)) + act(() => last?.dismiss('http://a.com')) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) + + // switching conversations unmounts this subtree: the provider is keyed on the + // conversation, so coming back mounts a fresh hook whose first render has no text yet + first.unmount() + render( (last = r)} />) + act(() => { + jest.advanceTimersByTime(500) + }) + expect(getSuppressedURLs(convID)).toEqual(['http://a.com']) + }) +}) diff --git a/shared/chat/conversation/unfurl-preview-state.tsx b/shared/chat/conversation/unfurl-preview-state.tsx new file mode 100644 index 000000000000..8cf431f390f9 --- /dev/null +++ b/shared/chat/conversation/unfurl-preview-state.tsx @@ -0,0 +1,227 @@ +import * as React from 'react' +import * as T from '@/constants/types' +import * as Z from '@/util/zustand' +import {ignorePromise} from '@/constants/utils' +import logger from '@/logger' + +type State = T.Immutable<{ + dismissed: Map> + // urls the service could not preview. they suppress on send like a dismissal does, but + // are kept apart from one: the next fetch replaces this set wholesale, and a url that + // starts scraping again must come back as a card, which it could not do if a failure + // had been recorded as something the user dismissed, since nothing re-derives a dismissal + failed: Map> + dispatch: { + dismiss: (conversationIDKey: T.Chat.ConversationIDKey, urls: ReadonlyArray) => void + keepOnly: (conversationIDKey: T.Chat.ConversationIDKey, urls: ReadonlyArray) => void + remove: (conversationIDKey: T.Chat.ConversationIDKey, urls: ReadonlyArray) => void + setFailed: (conversationIDKey: T.Chat.ConversationIDKey, urls: ReadonlyArray) => void + resetState: () => void + } +}> + +export const useUnfurlPreviewState = Z.createZustand('unfurl-preview', set => ({ + dismissed: new Map(), + failed: new Map(), + dispatch: { + // also the restore path after a canceled send: both mean "these urls are suppressed" + dismiss: (conversationIDKey, urls) => { + if (!urls.length) return + set(s => { + const existing = s.dismissed.get(conversationIDKey) ?? new Set() + for (const url of urls) existing.add(url) + s.dismissed.set(conversationIDKey, existing) + }) + }, + keepOnly: (conversationIDKey, urls) => { + set(s => { + const existing = s.dismissed.get(conversationIDKey) + if (!existing) return + for (const url of [...existing]) { + if (!urls.includes(url)) existing.delete(url) + } + if (!existing.size) s.dismissed.delete(conversationIDKey) + }) + }, + setFailed: (conversationIDKey, urls) => { + set(s => { + if (!urls.length) { + s.failed.delete(conversationIDKey) + return + } + s.failed.set(conversationIDKey, new Set(urls)) + }) + }, + remove: (conversationIDKey, urls) => { + set(s => { + const existing = s.dismissed.get(conversationIDKey) + if (!existing) return + for (const url of urls) existing.delete(url) + if (!existing.size) s.dismissed.delete(conversationIDKey) + }) + }, + resetState: Z.defaultReset, + }, +})) + +// the two suppression sources kept apart, because a send that never posts has to put the +// dismissals back without the failures: see the `failed` field above +export type SuppressSnapshot = T.Immutable<{dismissed: ReadonlyArray; failed: ReadonlyArray}> + +export const takeSuppressSnapshot = (conversationIDKey: T.Chat.ConversationIDKey): SuppressSnapshot => { + const {dismissed, failed} = useUnfurlPreviewState.getState() + return { + dismissed: [...(dismissed.get(conversationIDKey) ?? [])], + failed: [...(failed.get(conversationIDKey) ?? [])], + } +} + +// the send suppresses both, so the message unfurls exactly the cards the composer offered +export const suppressedURLsOf = (snapshot: SuppressSnapshot) => [ + ...new Set([...snapshot.dismissed, ...snapshot.failed]), +] + +// dropped once the send they belong to lands; a targeted remove rather than a +// whole-conversation clear so a dismissal made while that send was in flight survives +export const removeDismissals = (conversationIDKey: T.Chat.ConversationIDKey, urls: ReadonlyArray) => { + useUnfurlPreviewState.getState().dispatch.remove(conversationIDKey, urls) +} + +// put back what a send took when that send never posted, so the composer the user gets +// back still has those urls dismissed +export const restoreDismissals = (conversationIDKey: T.Chat.ConversationIDKey, urls: ReadonlyArray) => { + useUnfurlPreviewState.getState().dispatch.dismiss(conversationIDKey, urls) +} + +// a send during this window (paste a link, hit enter) suppresses nothing, so the message +// unfurls the url the way it always has: the composer only promises to show what will +// unfurl once its previews have landed. suppressing urls it has not heard about yet would +// mean a link sent quickly never unfurls at all, which is how most links go out +const debounceMS = 500 + +// what can follow a url and still end it. typing on past one makes the old url a prefix of +// the new one, and a plain substring test would keep the stale card alive and dismissable +// while the message carries a different link +const urlEnd = /[\s.,;:!?)\]}'"]/ + +const stillInText = (text: string, url: string) => { + for (let from = text.indexOf(url); from >= 0; from = text.indexOf(url, from + 1)) { + const after = text[from + url.length] + if (after === undefined || urlEnd.test(after)) return true + } + return false +} + +// kept outside the hook body: try/catch inside a hook trips the react-compiler bailout check +const fetchPreviews = async ( + conversationIDKey: T.Chat.ConversationIDKey, + text: string, + requestID: number, + requestIDRef: {current: number}, + onSuccess: (conversationIDKey: T.Chat.ConversationIDKey, infos: ReadonlyArray) => void +) => { + try { + const res = await T.RPCChat.localUnfurlPreviewLocalRpcPromise({ + convID: T.Chat.keyToConversationID(conversationIDKey), + text, + }) + if (requestID !== requestIDRef.current) return + onSuccess(conversationIDKey, res ?? []) + } catch (e) { + // best-effort preview: nothing is shown for a url whose fetch failed, since `visible` + // only surfaces previews whose url is still in the composer text + logger.info('unfurl preview failed', e) + } +} + +// module-wide, so an id is never handed out twice. a per-mount counter would start over, +// and a mount is torn down and rebuilt without a fresh ref whenever a screen freezes, which +// would let a fetch left running by the old one match the new one's id +let nextRequestID = 0 +// through a helper: the compiler bails out of a hook that updates a global in place +const takeRequestID = () => ++nextRequestID + +export const useUnfurlPreviews = (conversationIDKey: T.Chat.ConversationIDKey, text: string) => { + const [fetched, setFetched] = React.useState>([]) + const dismissedSet = useUnfurlPreviewState(s => s.dismissed.get(conversationIDKey)) + const {dismiss: dismissURL, keepOnly, setFailed} = useUnfurlPreviewState(s => s.dispatch) + // the id of the only fetch whose result this mount will still take + const requestIDRef = React.useRef(0) + // the input subtree remounts per conversation (key={conversationIDKey} on the provider), + // so the first render of a conversation we return to always has empty text before the + // draft is restored. clearing dismissals on that would throw away what the user + // dismissed before switching away, so only prune once real text has been seen. + const sawTextRef = React.useRef(false) + const hasLink = text.includes('http') + + // retires every fetch this mount left in flight, so one cannot write into the mount that + // replaces it. 0 is not a request id, and ids are never reused, so nothing can match again + React.useEffect(() => { + // the alias is for the lint rule about reading a ref in cleanup: this ref holds a + // request id, not a node, and retiring it is the whole point of the cleanup + const requests = requestIDRef + return () => { + requests.current = 0 + } + }, []) + + const onFetched = React.useCallback( + (fetchedConversationIDKey: T.Chat.ConversationIDKey, infos: ReadonlyArray) => { + setFetched(infos) + keepOnly( + fetchedConversationIDKey, + infos.map(i => i.url) + ) + setFailed( + fetchedConversationIDKey, + infos.filter(i => !i.unfurl).map(i => i.url) + ) + }, + [keepOnly, setFailed] + ) + + React.useEffect(() => { + const id = takeRequestID() + requestIDRef.current = id + if (!hasLink) { + if (sawTextRef.current) { + keepOnly(conversationIDKey, []) + setFailed(conversationIDKey, []) + } + sawTextRef.current = sawTextRef.current || !!text + return + } + sawTextRef.current = true + const timeoutID = setTimeout(() => { + ignorePromise(fetchPreviews(conversationIDKey, text, id, requestIDRef, onFetched)) + }, debounceMS) + return () => { + clearTimeout(timeoutID) + } + }, [conversationIDKey, hasLink, keepOnly, setFailed, text, onFetched]) + + const dismiss = React.useCallback( + (url: string) => { + dismissURL(conversationIDKey, [url]) + }, + [conversationIDKey, dismissURL] + ) + + // a card is only shown while its url is still in the composer. the fetch that would + // replace these previews can fail or still be in flight, and showing a card for a url the + // user has since deleted is worse than showing nothing: its X would suppress a link that + // is not in the message, while the link that is about to send never gets offered one. + // no unfurl means the service could not preview it: nothing to show, nothing to dismiss + const visible = React.useMemo( + () => + hasLink + ? fetched.flatMap(p => + p.unfurl && stillInText(text, p.url) && !dismissedSet?.has(p.url) + ? [{unfurl: p.unfurl, url: p.url}] + : [] + ) + : [], + [hasLink, fetched, text, dismissedSet] + ) + return {dismiss, previews: visible} +} diff --git a/shared/constants/rpc/rpc-chat-gen.tsx b/shared/constants/rpc/rpc-chat-gen.tsx index ef96791cd603..3fac088cb733 100644 --- a/shared/constants/rpc/rpc-chat-gen.tsx +++ b/shared/constants/rpc/rpc-chat-gen.tsx @@ -496,7 +496,7 @@ export type MessageTypes = { outParam: PostLocalNonblockRes, }, 'chat.1.local.postTextNonblock': { - inParam: {readonly conversationID: ConversationID,readonly tlfName: string,readonly tlfPublic: boolean,readonly body: string,readonly clientPrev: MessageID,readonly replyTo?: MessageID | null,readonly outboxID?: OutboxID | null,readonly identifyBehavior: Keybase1.TLFIdentifyBehavior,readonly ephemeralLifetime?: Gregor1.DurationSec | null}, + inParam: {readonly conversationID: ConversationID,readonly tlfName: string,readonly tlfPublic: boolean,readonly body: string,readonly clientPrev: MessageID,readonly replyTo?: MessageID | null,readonly outboxID?: OutboxID | null,readonly identifyBehavior: Keybase1.TLFIdentifyBehavior,readonly ephemeralLifetime?: Gregor1.DurationSec | null,readonly unfurlSuppress?: ReadonlyArray | null}, outParam: PostLocalNonblockRes, }, 'chat.1.local.previewConversationByIDLocal': { @@ -607,6 +607,10 @@ export type MessageTypes = { inParam: {readonly payload: string,readonly convID: string,readonly membersType: ConversationMembersType}, outParam: string, }, + 'chat.1.local.unfurlPreviewLocal': { + inParam: {readonly convID: ConversationID,readonly text: string}, + outParam: ReadonlyArray | null, + }, 'chat.1.local.unpinMessage': { inParam: {readonly convID: ConversationID}, outParam: PinMessageRes, @@ -628,7 +632,7 @@ export type MessageKey = keyof MessageTypes export type RpcIn = MessageTypes[M]['inParam'] export type RpcOut = MessageTypes[M]['outParam'] export type RpcResponse = {error: IncomingErrorCallback, result: (res: RpcOut) => void} -type PromiseMethod = 'chat.1.local.addBotConvSearch' | 'chat.1.local.addBotMember' | 'chat.1.local.addEmojiAlias' | 'chat.1.local.addEmojis' | 'chat.1.local.addTeamMemberAfterReset' | 'chat.1.local.archiveChat' | 'chat.1.local.archiveChatDelete' | 'chat.1.local.archiveChatList' | 'chat.1.local.archiveChatPause' | 'chat.1.local.archiveChatResume' | 'chat.1.local.bulkAddToConv' | 'chat.1.local.bulkAddToManyConvs' | 'chat.1.local.cancelActiveInboxSearch' | 'chat.1.local.cancelActiveSearch' | 'chat.1.local.CancelPost' | 'chat.1.local.cancelUploadTempFile' | 'chat.1.local.ConfigureFileAttachmentDownloadLocal' | 'chat.1.local.deleteConversationLocal' | 'chat.1.local.dismissJourneycard' | 'chat.1.local.DownloadFileAttachmentLocal' | 'chat.1.local.findConversationsLocal' | 'chat.1.local.findGeneralConvFromTeamID' | 'chat.1.local.forwardMessageConvSearch' | 'chat.1.local.forwardMessageNonblock' | 'chat.1.local.getBotMemberSettings' | 'chat.1.local.getDefaultTeamChannelsLocal' | 'chat.1.local.getGlobalAppNotificationSettingsLocal' | 'chat.1.local.getInboxAndUnboxUILocal' | 'chat.1.local.getLastActiveAtMultiLocal' | 'chat.1.local.getLastActiveForTeams' | 'chat.1.local.getMutualTeamsLocal' | 'chat.1.local.getNextAttachmentMessageLocal' | 'chat.1.local.getRecentJoinsLocal' | 'chat.1.local.getStaticConfig' | 'chat.1.local.getTeamRetentionLocal' | 'chat.1.local.getTeamRoleInConversation' | 'chat.1.local.getTLFConversationsLocal' | 'chat.1.local.getUnfurlSettings' | 'chat.1.local.getUnreadline' | 'chat.1.local.getUploadTempFile' | 'chat.1.local.getWelcomeMessage' | 'chat.1.local.ignorePinnedMessage' | 'chat.1.local.joinConversationByIDLocal' | 'chat.1.local.leaveConversationLocal' | 'chat.1.local.listPublicBotCommandsLocal' | 'chat.1.local.locationUpdate' | 'chat.1.local.makeAudioPreview' | 'chat.1.local.makeUploadTempFile' | 'chat.1.local.markAsReadLocal' | 'chat.1.local.markTLFAsReadLocal' | 'chat.1.local.newConversationLocal' | 'chat.1.local.pinMessage' | 'chat.1.local.postDeleteHistoryByAge' | 'chat.1.local.postDeleteNonblock' | 'chat.1.local.postEditNonblock' | 'chat.1.local.postFileAttachmentLocalNonblock' | 'chat.1.local.postHeadline' | 'chat.1.local.postHeadlineNonblock' | 'chat.1.local.postMetadata' | 'chat.1.local.postReactionNonblock' | 'chat.1.local.previewConversationByIDLocal' | 'chat.1.local.putReacjiSkinTone' | 'chat.1.local.refreshParticipants' | 'chat.1.local.removeBotMember' | 'chat.1.local.removeEmoji' | 'chat.1.local.removeFromConversationLocal' | 'chat.1.local.requestInboxLayout' | 'chat.1.local.requestInboxSmallIncrease' | 'chat.1.local.requestInboxSmallReset' | 'chat.1.local.requestInboxUnbox' | 'chat.1.local.resolveMaybeMention' | 'chat.1.local.resolveUnfurlPrompt' | 'chat.1.local.RetryPost' | 'chat.1.local.saveUnfurlSettings' | 'chat.1.local.setAppNotificationSettingsLocal' | 'chat.1.local.setBotMemberSettings' | 'chat.1.local.SetConversationStatusLocal' | 'chat.1.local.setConvMinWriterRoleLocal' | 'chat.1.local.setConvRetentionLocal' | 'chat.1.local.setDefaultTeamChannelsLocal' | 'chat.1.local.setGlobalAppNotificationSettingsLocal' | 'chat.1.local.setTeamRetentionLocal' | 'chat.1.local.setWelcomeMessage' | 'chat.1.local.simpleSearchInboxConvNames' | 'chat.1.local.toggleEmojiAnimations' | 'chat.1.local.toggleMessageCollapse' | 'chat.1.local.trackGiphySelect' | 'chat.1.local.unboxMobilePushNotification' | 'chat.1.local.unpinMessage' | 'chat.1.local.updateTyping' | 'chat.1.local.updateUnsentText' | 'chat.1.local.userEmojis' +type PromiseMethod = 'chat.1.local.addBotConvSearch' | 'chat.1.local.addBotMember' | 'chat.1.local.addEmojiAlias' | 'chat.1.local.addEmojis' | 'chat.1.local.addTeamMemberAfterReset' | 'chat.1.local.archiveChat' | 'chat.1.local.archiveChatDelete' | 'chat.1.local.archiveChatList' | 'chat.1.local.archiveChatPause' | 'chat.1.local.archiveChatResume' | 'chat.1.local.bulkAddToConv' | 'chat.1.local.bulkAddToManyConvs' | 'chat.1.local.cancelActiveInboxSearch' | 'chat.1.local.cancelActiveSearch' | 'chat.1.local.CancelPost' | 'chat.1.local.cancelUploadTempFile' | 'chat.1.local.ConfigureFileAttachmentDownloadLocal' | 'chat.1.local.deleteConversationLocal' | 'chat.1.local.dismissJourneycard' | 'chat.1.local.DownloadFileAttachmentLocal' | 'chat.1.local.findConversationsLocal' | 'chat.1.local.findGeneralConvFromTeamID' | 'chat.1.local.forwardMessageConvSearch' | 'chat.1.local.forwardMessageNonblock' | 'chat.1.local.getBotMemberSettings' | 'chat.1.local.getDefaultTeamChannelsLocal' | 'chat.1.local.getGlobalAppNotificationSettingsLocal' | 'chat.1.local.getInboxAndUnboxUILocal' | 'chat.1.local.getLastActiveAtMultiLocal' | 'chat.1.local.getLastActiveForTeams' | 'chat.1.local.getMutualTeamsLocal' | 'chat.1.local.getNextAttachmentMessageLocal' | 'chat.1.local.getRecentJoinsLocal' | 'chat.1.local.getStaticConfig' | 'chat.1.local.getTeamRetentionLocal' | 'chat.1.local.getTeamRoleInConversation' | 'chat.1.local.getTLFConversationsLocal' | 'chat.1.local.getUnfurlSettings' | 'chat.1.local.getUnreadline' | 'chat.1.local.getUploadTempFile' | 'chat.1.local.getWelcomeMessage' | 'chat.1.local.ignorePinnedMessage' | 'chat.1.local.joinConversationByIDLocal' | 'chat.1.local.leaveConversationLocal' | 'chat.1.local.listPublicBotCommandsLocal' | 'chat.1.local.locationUpdate' | 'chat.1.local.makeAudioPreview' | 'chat.1.local.makeUploadTempFile' | 'chat.1.local.markAsReadLocal' | 'chat.1.local.markTLFAsReadLocal' | 'chat.1.local.newConversationLocal' | 'chat.1.local.pinMessage' | 'chat.1.local.postDeleteHistoryByAge' | 'chat.1.local.postDeleteNonblock' | 'chat.1.local.postEditNonblock' | 'chat.1.local.postFileAttachmentLocalNonblock' | 'chat.1.local.postHeadline' | 'chat.1.local.postHeadlineNonblock' | 'chat.1.local.postMetadata' | 'chat.1.local.postReactionNonblock' | 'chat.1.local.previewConversationByIDLocal' | 'chat.1.local.putReacjiSkinTone' | 'chat.1.local.refreshParticipants' | 'chat.1.local.removeBotMember' | 'chat.1.local.removeEmoji' | 'chat.1.local.removeFromConversationLocal' | 'chat.1.local.requestInboxLayout' | 'chat.1.local.requestInboxSmallIncrease' | 'chat.1.local.requestInboxSmallReset' | 'chat.1.local.requestInboxUnbox' | 'chat.1.local.resolveMaybeMention' | 'chat.1.local.resolveUnfurlPrompt' | 'chat.1.local.RetryPost' | 'chat.1.local.saveUnfurlSettings' | 'chat.1.local.setAppNotificationSettingsLocal' | 'chat.1.local.setBotMemberSettings' | 'chat.1.local.SetConversationStatusLocal' | 'chat.1.local.setConvMinWriterRoleLocal' | 'chat.1.local.setConvRetentionLocal' | 'chat.1.local.setDefaultTeamChannelsLocal' | 'chat.1.local.setGlobalAppNotificationSettingsLocal' | 'chat.1.local.setTeamRetentionLocal' | 'chat.1.local.setWelcomeMessage' | 'chat.1.local.simpleSearchInboxConvNames' | 'chat.1.local.toggleEmojiAnimations' | 'chat.1.local.toggleMessageCollapse' | 'chat.1.local.trackGiphySelect' | 'chat.1.local.unboxMobilePushNotification' | 'chat.1.local.unfurlPreviewLocal' | 'chat.1.local.unpinMessage' | 'chat.1.local.updateTyping' | 'chat.1.local.updateUnsentText' | 'chat.1.local.userEmojis' export type RpcFn = [RpcIn] extends [undefined] ? (params?: undefined, waitingKey?: WaitingKey) => Promise> : (params: RpcIn, waitingKey?: WaitingKey) => Promise> @@ -1472,7 +1476,7 @@ export type SearchOpts = {readonly isRegex: boolean,readonly sentBy: string,read export type SearchRegexpRes = {readonly offline: boolean,readonly hits?: ReadonlyArray | null,readonly rateLimits?: ReadonlyArray | null,readonly identifyFailures?: ReadonlyArray | null,} export type SendRes = {readonly message: string,readonly messageID?: MessageID | null,readonly outboxID?: OutboxID | null,readonly identifyFailures?: ReadonlyArray | null,readonly rateLimits?: ReadonlyArray | null,} export type SenderPrepareOptions = {readonly skipTopicNameState: boolean,readonly replyTo?: MessageID | null,} -export type SenderSendOptions = {readonly joinMentionsAs?: ConversationMemberStatus | null,} +export type SenderSendOptions = {readonly joinMentionsAs?: ConversationMemberStatus | null,readonly unfurlSuppress?: ReadonlyArray | null,} export type ServerCacheVers = {readonly inboxVers: number,readonly bodiesVers: number,} export type ServerNowRes = {readonly rateLimit?: RateLimit | null,readonly now: Gregor1.Time,} export type SetAppNotificationSettingsInfo = {readonly convID: ConversationID,readonly settings: ConversationNotificationInfo,} @@ -1574,6 +1578,7 @@ export type UnfurlGiphyDisplay = {readonly favicon?: UnfurlImageDisplay | null,r export type UnfurlGiphyRaw = {readonly imageUrl?: string | null,readonly video?: UnfurlVideo | null,readonly faviconUrl?: string | null,} export type UnfurlImageDisplay = {readonly url: string,readonly height: number,readonly width: number,readonly isVideo: boolean,} export type UnfurlMapsRaw = {readonly title: string,readonly url: string,readonly siteName: string,readonly imageUrl: string,readonly historyImageUrl?: string | null,readonly description: string,readonly coord: Coordinate,readonly time: Gregor1.Time,readonly liveLocationEndTime?: Gregor1.Time | null,readonly liveLocationDone: boolean,} +export type UnfurlPreviewInfo = {readonly url: string,readonly unfurl?: UnfurlDisplay | null,} export type UnfurlPromptResult ={ actionType: UnfurlPromptAction.always } | { actionType: UnfurlPromptAction.never } | { actionType: UnfurlPromptAction.notnow } | { actionType: UnfurlPromptAction.accept, accept: string } | { actionType: UnfurlPromptAction.onetime, onetime: string } export type UnfurlRaw ={ unfurlType: UnfurlType.generic, generic: UnfurlGenericRaw } | { unfurlType: UnfurlType.youtube, youtube: UnfurlYoutubeRaw } | { unfurlType: UnfurlType.giphy, giphy: UnfurlGiphyRaw } | { unfurlType: UnfurlType.maps, maps: UnfurlMapsRaw } export type UnfurlResult = {readonly unfurl: Unfurl,readonly url: string,} @@ -1699,6 +1704,7 @@ export const localToggleEmojiAnimationsRpcPromise = createRpc('chat.1.local.togg export const localToggleMessageCollapseRpcPromise = createRpc('chat.1.local.toggleMessageCollapse') export const localTrackGiphySelectRpcPromise = createRpc('chat.1.local.trackGiphySelect') export const localUnboxMobilePushNotificationRpcPromise = createRpc('chat.1.local.unboxMobilePushNotification') +export const localUnfurlPreviewLocalRpcPromise = createRpc('chat.1.local.unfurlPreviewLocal') export const localUnpinMessageRpcPromise = createRpc('chat.1.local.unpinMessage') export const localUpdateTypingRpcPromise = createRpc('chat.1.local.updateTyping') export const localUpdateUnsentTextRpcPromise = createRpc('chat.1.local.updateUnsentText') diff --git a/shared/tsconfig.native.json b/shared/tsconfig.native.json index 399783d67966..3ac69ddc1e79 100644 --- a/shared/tsconfig.native.json +++ b/shared/tsconfig.native.json @@ -18,6 +18,7 @@ "./common-adapters/icon.constants-gen.native.tsx", "./common-adapters/icon.constants-gen.shared.tsx", "./chat/conversation/normal/container.test.tsx", + "./chat/conversation/input-area/unfurl-preview.test.tsx", "./chat/conversation/messages/system-users-added-to-conv/container.test.tsx", "./chat/conversation/messages/text/coinflip/results.test.tsx", "./common-adapters/markdown/index.test.tsx",