search

yuya373 / emacs-slack

slack client for emacs
1.1k stars 117 forks source link

Long links don't work #547

Open ieure opened 3 years ago

ieure commented 3 years ago

Describe the bug

When someone posts a long link, Slack removes some of the middle, replacing it with […].

For the original link:

https://markets.businessinsider.com/news/stocks/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-1030136284

Slack turns this into:

https://markets.businessinsider.com/news/stocks/dailypay-expands-platform-announces-integr[…]an-capital-management-and-payroll-industries-1030136284

If I move point to the portion of the link before the […] and press RET, it opens the portion of the URL up to the […]:

https://markets.businessinsider.com/news/stocks/dailypay-expands-platform-announces-integr

If I put point after the […] and press RET, it works fine.

This is how it looks in the *Slack Log* buffer:

(:ok t :messages ((:client_msg_id "3e999af5-63ec-4e3b-b663-b54a9abb29a1" :type "message" :text "<https://www.prnewswire.com/news-releases/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-301237466.html|https://www.prnewswire.com/news-releases/dailypay-expands-platform-announces-integrati[…]an-capital-management-and-payroll-industries-301237466.html>" :user "U8LEUHE9L" :ts "1614707855.007300" :team "T02F87F55" :attachments ((:title "DailyPay Expands Platform: Announces Integration Offering For Human Capital Management And Payroll Industries" :title_link "https://www.prnewswire.com/news-releases/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-301237466.html" :text "/PRNewswire/ -- DailyPay, the recognized gold standard in the on-demand pay industry, today announced the launch of a game-changing new product, ExtendPX, for..." :fallback "DailyPay Expands Platform: Announces Integration Offering For Human Capital Management And Payroll Industries" :image_url "https://mma.prnewswire.com/media/1445852/dailypay_logo_business_blk__4_Logo.jpg?p=twitter" :from_url "https://www.prnewswire.com/news-releases/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-301237466.html" :image_width 885 :image_height 250 :image_bytes 31183 :service_name "prnewswire.com" :id 1 :original_url "https://www.prnewswire.com/news-releases/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-301237466.html" :fields nil :actions nil :files nil)) :blocks ((:type "rich_text" :block_id "2AgK" :elements ((:type "rich_text_section" :elements ((:type "link" :url "https://www.prnewswire.com/news-releases/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-301237466.html" :text "https://www.prnewswire.com/news-releases/dailypay-expands-platform-announces-integrati[…]an-capital-management-and-payroll-industries-301237466.html")))))) :thread_ts "1614707855.007300" :reply_count 1 :reply_users_count 1 :latest_reply "1614707879.007500" :reply_users ("U8LEUHE9L") :subscribed :json-false :reactions nil :pinned_to nil :channel "CBLCPJD0R") (:client_msg_id "1bd5dbe3-0707-4e8e-aa8f-dc226b7cdb74" :type "message" :text "<https://markets.businessinsider.com/news/stocks/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-1030136284|https://markets.businessinsider.com/news/stocks/dailypay-expands-platform-announces-integr[…]an-capital-management-and-payroll-industries-1030136284>" :user "U8LEUHE9L" :ts "1614707879.007500" :team "T02F87F55" :blocks ((:type "rich_text" :block_id "oFg" :elements ((:type "rich_text_section" :elements ((:type "link" :url "https://markets.businessinsider.com/news/stocks/dailypay-expands-platform-announces-integration-offering-for-human-capital-management-and-payroll-industries-1030136284" :text "https://markets.businessinsider.com/news/stocks/dailypay-expands-platform-announces-integr[…]an-capital-management-and-payroll-industries-1030136284")))))) :thread_ts "1614707855.007300" :parent_user_id "U8LEUHE9L" :reactions nil :attachments nil :pinned_to nil :channel "CBLCPJD0R")

This seems to generate two overlays, one with broken arguments and one with correct ones:

There are 2 overlays here:
 From 624 to 714
  action               lui-button-activate
  button               [Show]
  category             lui-button-button
  lui-button-arguments [Show]
  lui-button-function  browse-url
 From 624 to 772
  action               lui-button-activate
  button               [Show]
  category             lui-button-button
  lui-button-arguments [Show]
  lui-button-function  browse-url

To Reproduce Steps to reproduce the behavior:

  1. Wait for someone to post a long link with […] in the middle.
  2. Click the first part of it.

Backtrace

Paste Backtrace if any.
Do not forget to mask confidential information. (e.g. token, message, ...)

Expected behavior

The right link should open.

Environment:

Additional context

jumper047 commented 1 year ago

Fun fact - actually they work if you activate it clicking on part to the right of the [...] text.

ieure commented 1 year ago

Fun fact - actually they work if you activate it clicking on part to the right of the [...] text.

Fun fact - this is already in the bug report.

jumper047 commented 1 year ago

Your fact is much funnier then mine, you win (sorry, seems like I read just two lines below title)

jumper047 commented 1 year ago

Seems like I found the root of that issue - url button created twice, first time by slack and then by lui-buttonize-urls function. Quick fix - add to your config (advice-add 'lui-buttonize-urls :before-until (lambda () (derived-mode-p 'slack-mode)))