Bug in spoiler iOS notifications

dfabulich · April 9, 2019, 3:59am

There’s a bug in the way the Discourse Hub iOS app handles spoilers; it shows the spoiler text in the clear.

To reproduce: Install the Discourse Hub iOS app, sign in to a forum, and sign up for notifications.

Use a separate account to send yourself a private message containing a blurred spoiler, like this. You’ll receive a push notification of the PM.

Actual:

Expected: The spoiler text should be concealed.

dfabulich · April 9, 2019, 4:00am

I think this is a regression, but I’m not 100% sure of it.

pmusaraj · April 10, 2019, 1:55am

I’m going to look into this since I am doing some other work around iOS and push notifications.

zogstrip · April 10, 2019, 6:48am

I think it comes from our excerpt parser that is being called here

github.com/discourse/discourse

app/services/post_alerter.rb

b58867b6e


      
            created.id ? created : nil
          end
          
          def create_notification_alert(user:, post:, notification_type:, excerpt: nil, username: nil)
            if post_url = post.url
              payload = {
               notification_type: notification_type,
               post_number: post.post_number,
               topic_title: post.topic.title,
               topic_id: post.topic.id,
               excerpt: excerpt || post.excerpt(400, text_entities: true, strip_links: true, remap_emoji: true),
               username: username || post.username,
               post_url: post_url
              }
          
              MessageBus.publish("/notification-alert/#{user.id}", payload, user_ids: [user.id])
              push_notification(user, payload)
              DiscourseEvent.trigger(:post_notification_alert, user, payload)
            end
          end

We have some code to preserve spoilers but I don’t know why we did that and it seems like a bad idea

github.com/discourse/discourse

lib/excerpt_parser.rb

b58867b6e


      
          # Preserve spoilers
          if attributes.include?(["class", "spoiler"])
            include_tag("span", attributes)
            @in_spoiler = true
          end

We should be replacing the whole spoiler with [spoiler] like we do for images.

pmusaraj · April 12, 2019, 4:02pm

Thanks @zogstrip, that was very helpful. Fixed in FIX: strip spoilers in notification excerpts · discourse/discourse@5594222 · GitHub

Topic		Replies	Views
Spoiler text in emails is simply missing Bug	4	3077	April 9, 2019
Spoiler issues with VoiceOver Bug accessibility , spoiler-alert , screen-reader	14	1857	August 1, 2023
Discourse Spoiler-Alert Plugin official , spoiler-alert , included-in-core	12	36675	September 1, 2026
Rich preview for Push Notification on iOS Support push-notifications	0	117	April 11, 2025
iOS app: viewing first forum erases notifications of second Bug discourse-hub	26	2073	November 26, 2023

Bug in spoiler iOS notifications

AI-generated summary

Related topics