Image refactor refactor (#541)

* Remove outdated comments about srcSet value We no longer distinguish between `undefined` and `null` for `srcSet`. * Fix misleading URL shown * Update/fix comments in <ImageOriginal> * Simplify condition when to show image I think this is not required since `showImage` will always stay false if tab !== 'preview' or me?.clickToLoadImg are true. * Rename to imgproxyOnly * Add info to imgproxyOnly setting * Fix text of markdown links not used on imgproxy errors * Fix rendering markdown links with text as images --------- Co-authored-by: ekzyis <[email protected]>
stackernews · Oct 3, 2023 · 1e417ba · 1e417ba
1 parent 33c1add
commit 1e417ba
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 19 deletions.
diff --git a/api/typeDefs/user.js b/api/typeDefs/user.js
@@ -23,7 +23,7 @@ export default gql`
     setSettings(tipDefault: Int!, turboTipping: Boolean!, fiatCurrency: String!, noteItemSats: Boolean!,
       noteEarning: Boolean!, noteAllDescendants: Boolean!, noteMentions: Boolean!, noteDeposits: Boolean!,
       noteInvites: Boolean!, noteJobIndicator: Boolean!, noteCowboyHat: Boolean!, hideInvoiceDesc: Boolean!,
-      hideFromTopUsers: Boolean!, hideCowboyHat: Boolean!, clickToLoadImg: Boolean!,
+      hideFromTopUsers: Boolean!, hideCowboyHat: Boolean!, imgproxyOnly: Boolean!,
       wildWestMode: Boolean!, greeterMode: Boolean!, nostrPubkey: String, nostrRelays: [String!], hideBookmarks: Boolean!,
       noteForwardedSats: Boolean!, hideWalletBalance: Boolean!, hideIsContributor: Boolean!, diagnostics: Boolean!): User
     setPhoto(photoId: ID!): Int!
@@ -89,7 +89,7 @@ export default gql`
     hideWelcomeBanner: Boolean!
     hideWalletBalance: Boolean!
     diagnostics: Boolean!
-    clickToLoadImg: Boolean!
+    imgproxyOnly: Boolean!
     wildWestMode: Boolean!
     greeterMode: Boolean!
     lastCheckedJobs: String

diff --git a/components/image.js b/components/image.js
@@ -19,7 +19,7 @@ function ImageOriginal ({ src, topLevel, nofollow, tab, children, onClick, ...pr
   const [showImage, setShowImage] = useState(false)
 
   useEffect(() => {
-    if (me?.clickToLoadImg && tab !== 'preview') return
+    if (me?.imgproxyOnly && tab !== 'preview') return
     // make sure it's not a false negative by trying to load URL as <img>
     const img = new window.Image()
     img.onload = () => setShowImage(true)
@@ -31,8 +31,7 @@ function ImageOriginal ({ src, topLevel, nofollow, tab, children, onClick, ...pr
     }
   }, [src, showImage])
 
-  if (showImage && (tab === 'preview' || !me?.clickToLoadImg)) {
-    // image is still processing and user is okay with loading original url automatically
+  if (showImage) {
     return (
       <img
         className={topLevel ? styles.topLevel : undefined}
@@ -42,13 +41,17 @@ function ImageOriginal ({ src, topLevel, nofollow, tab, children, onClick, ...pr
       />
     )
   } else {
-    // image is still processing or user is not okay with loading original url automatically
+    // user is not okay with loading original url automatically or there was an error loading the image
+
+    // If element parsed by markdown is a raw URL, we use src as the text to not mislead users.
+    // This will not be the case if [text](url) format is used. Then we will show what was chosen as text.
+    const isRawURL = /^https?:\/\//.test(children?.[0])
     return (
       <a
         target='_blank'
         rel={`noreferrer ${nofollow ? 'nofollow' : ''} noopener`}
         href={src}
-      >{children || src}
+      >{isRawURL ? src : children}
       </a>
     )
   }
@@ -89,8 +92,8 @@ function ImageProxy ({ src, srcSet: srcSetObj, onClick, topLevel, onError, ...pr
 export function ZoomableImage ({ src, srcSet, ...props }) {
   const showModal = useShowModal()
 
-  // if `srcSet` is undefined, it means the image was not processed by worker yet
-  const [imgproxy, setImgproxy] = useState(srcSet || IMGPROXY_URL_REGEXP.test(src))
+  // if `srcSet` is falsy, it means the image was not processed by worker yet
+  const [imgproxy, setImgproxy] = useState(!!srcSet || IMGPROXY_URL_REGEXP.test(src))
 
   // backwards compatibility:
   // src may already be imgproxy url since we used to replace image urls with imgproxy urls

diff --git a/components/text.js b/components/text.js
@@ -101,8 +101,6 @@ export default memo(function Text ({ nofollow, imgproxyUrls, children, tab, ...o
 
   const Img = useCallback(({ node, src, ...props }) => {
     const url = IMGPROXY_URL_REGEXP.test(src) ? decodeOriginalUrl(src) : src
-    // if `srcSet` is undefined, it means the image was not processed by worker yet
-    // if `srcSet` is null, image was processed but this specific url was not detected as an image by the worker
     const srcSet = imgproxyUrls?.[url]
     return <ZoomableImage srcSet={srcSet} tab={tab} src={src} {...props} {...outerProps} />
   }, [imgproxyUrls, outerProps, tab])
@@ -126,6 +124,13 @@ export default memo(function Text ({ nofollow, imgproxyUrls, children, tab, ...o
               return <>{children}</>
             }
 
+            // If [text](url) was parsed as <a> and text is not empty and not a link itself,
+            // we don't render it as an image since it was probably a concious choice to include text.
+            const text = children?.[0]
+            if (!!text && !/^https?:\/\//.test(text)) {
+              return <a target='_blank' rel={`noreferrer ${nofollow ? 'nofollow' : ''} noopener`} href={href}>{text}</a>
+            }
+
             // assume the link is an image which will fallback to link if it's not
             return <Img src={href} nofollow={nofollow} {...props}>{children}</Img>
           },

diff --git a/fragments/users.js b/fragments/users.js
@@ -30,7 +30,7 @@ export const ME = gql`
       hideInvoiceDesc
       hideFromTopUsers
       hideCowboyHat
-      clickToLoadImg
+      imgproxyOnly
       diagnostics
       wildWestMode
       greeterMode
@@ -61,7 +61,7 @@ export const SETTINGS_FIELDS = gql`
     hideCowboyHat
     hideBookmarks
     hideIsContributor
-    clickToLoadImg
+    imgproxyOnly
     hideWalletBalance
     diagnostics
     nostrPubkey
@@ -91,14 +91,14 @@ ${SETTINGS_FIELDS}
 mutation setSettings($tipDefault: Int!, $turboTipping: Boolean!, $fiatCurrency: String!, $noteItemSats: Boolean!,
   $noteEarning: Boolean!, $noteAllDescendants: Boolean!, $noteMentions: Boolean!, $noteDeposits: Boolean!,
   $noteInvites: Boolean!, $noteJobIndicator: Boolean!, $noteCowboyHat: Boolean!, $hideInvoiceDesc: Boolean!,
-  $hideFromTopUsers: Boolean!, $hideCowboyHat: Boolean!, $clickToLoadImg: Boolean!,
+  $hideFromTopUsers: Boolean!, $hideCowboyHat: Boolean!, $imgproxyOnly: Boolean!,
   $wildWestMode: Boolean!, $greeterMode: Boolean!, $nostrPubkey: String, $nostrRelays: [String!], $hideBookmarks: Boolean!,
   $noteForwardedSats: Boolean!, $hideWalletBalance: Boolean!, $hideIsContributor: Boolean!, $diagnostics: Boolean!) {
   setSettings(tipDefault: $tipDefault, turboTipping: $turboTipping,  fiatCurrency: $fiatCurrency,
     noteItemSats: $noteItemSats, noteEarning: $noteEarning, noteAllDescendants: $noteAllDescendants,
     noteMentions: $noteMentions, noteDeposits: $noteDeposits, noteInvites: $noteInvites,
     noteJobIndicator: $noteJobIndicator, noteCowboyHat: $noteCowboyHat, hideInvoiceDesc: $hideInvoiceDesc,
-    hideFromTopUsers: $hideFromTopUsers, hideCowboyHat: $hideCowboyHat, clickToLoadImg: $clickToLoadImg,
+    hideFromTopUsers: $hideFromTopUsers, hideCowboyHat: $hideCowboyHat, imgproxyOnly: $imgproxyOnly,
     wildWestMode: $wildWestMode, greeterMode: $greeterMode, nostrPubkey: $nostrPubkey, nostrRelays: $nostrRelays, hideBookmarks: $hideBookmarks,
     noteForwardedSats: $noteForwardedSats, hideWalletBalance: $hideWalletBalance, hideIsContributor: $hideIsContributor, diagnostics: $diagnostics) {
       ...SettingsFields

diff --git a/pages/settings.js b/pages/settings.js
@@ -75,7 +75,7 @@ export default function Settings ({ ssrData }) {
             hideInvoiceDesc: settings?.hideInvoiceDesc,
             hideFromTopUsers: settings?.hideFromTopUsers,
             hideCowboyHat: settings?.hideCowboyHat,
-            clickToLoadImg: settings?.clickToLoadImg,
+            imgproxyOnly: settings?.imgproxyOnly,
             wildWestMode: settings?.wildWestMode,
             greeterMode: settings?.greeterMode,
             nostrPubkey: settings?.nostrPubkey ? bech32encode(settings.nostrPubkey) : '',
@@ -254,8 +254,18 @@ export default function Settings ({ ssrData }) {
               groupClassName='mb-0'
             />}
           <Checkbox
-            label={<>click to load external images</>}
-            name='clickToLoadImg'
+            label={
+              <div className='d-flex align-items-center'>only load images from proxy
+                <Info>
+                  <ul className='fw-bold'>
+                    <li>only load images from our image proxy automatically</li>
+                    <li>this prevents IP address leaks to arbitrary sites</li>
+                    <li>if we fail to load an image, the raw link will be shown</li>
+                  </ul>
+                </Info>
+              </div>
+            }
+            name='imgproxyOnly'
             groupClassName='mb-0'
           />
           <Checkbox

diff --git a/prisma/migrations/20231003134505_rename_to_imgproxy_only/migration.sql b/prisma/migrations/20231003134505_rename_to_imgproxy_only/migration.sql
@@ -0,0 +1,2 @@
+-- AlterTable
+ALTER TABLE "users" RENAME COLUMN "clickToLoadImg" TO "imgproxyOnly";
diff --git a/prisma/schema.prisma b/prisma/schema.prisma
@@ -50,7 +50,7 @@ model User {
   fiatCurrency        String               @default("USD")
   hideFromTopUsers    Boolean              @default(false)
   turboTipping        Boolean              @default(false)
-  clickToLoadImg      Boolean              @default(false)
+  imgproxyOnly        Boolean              @default(false)
   hideWalletBalance   Boolean              @default(false)
   referrerId          Int?
   nostrPubkey         String?
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		-- AlterTable
		ALTER TABLE "users" RENAME COLUMN "clickToLoadImg" TO "imgproxyOnly";