Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adicionar rel="nofollow" nos links de conteúdos gerados por usuário #1603

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

Rafatcb
Copy link
Collaborator

@Rafatcb Rafatcb commented Jan 14, 2024

Mudanças realizadas

Adicionei o plugin rehype-external-links que serve exatamente para adicionar o rel="nofollow" em links externos. Essa bibliioteca recomenda usar rehype-sanitize para conteúdos gerados por usuários, mas o bytemd já faz isso antes de adicionar os plugins do rehype.

Coloquei areLinksTrusted nas páginas que nós controlamos o conteúdo. Os links de Fonte continuam sem o rel="nofollow" (acho que faz sentido, considerando que a Fonte seja usada da forma adequada, como "origem" do conteúdo).

Futuramente podemos criar alguma heurística para não adicionar o plugin em conteúdos de usuários confiáveis, ou usar a propriedade test para permitir domínios confiáveis (GitHub, por exemplo). Um exemplo de test que não colocará rel="nofollow" no link https://www.github.com:

pluginList.push(
  externalLinksPlugin({ test: (node) => node.properties?.href !== 'https://www.github.com' })
);

Do jeito que está no PR, o HTML gerado pelos links em publicações ficaram assim (em localhost):

Testes em publicação

Testes em comentário

Resolve #1236

Tipo de mudança

  • Nova funcionalidade

Checklist:

  • As modificações não geram novos logs de erro ou aviso (warning).
  • Eu adicionei testes que provam que a correção ou novo recurso funciona conforme esperado.
  • Tanto os novos testes quanto os antigos estão passando localmente.

@Rafatcb Rafatcb added front Envolve modificações no frontend novo recurso Nova funcionalidade/recurso SEO Otimização do site para motores de busca labels Jan 14, 2024
Copy link

vercel bot commented Jan 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tabnews ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 18, 2024 0:49am

Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muito bom Rafa!

Adicionei o plugin rehype-extenral-links que serve exatamente para adicionar o rel="nofollow" em links externos.

O plugin considera interno todos os links relativos, né? Só que está adicionando rel="nofollow" nos links de conteúdos criados pelos usuários com URLs absolutas do próprio TabNews. Será que isso não pode prejudicar o SEO?

Dá para passar uma função createRel como rel nas opções do rehypeExternalLinks, e devolver o array vazio quando forem links absolutos internos. Ou usar o test como você falou mais abaixo. Pode ser uma boa usar o webserver.host para comparar.

Essa bibliioteca recomenda usar rehype-sanitize para conteúdos gerados por usuários, mas o bytemd já faz isso antes de adicionar os plugins do rehype.

Inclusive nós já utilizamos um sanitize customizado:

function sanitize(defaultSchema) {
const schema = { ...defaultSchema };
schema.attributes['*'] = schema.attributes['*'].filter((attr) => attr != 'className');
schema.attributes['*'].push(['className', /^hljs|^language-|^bytemd-mermaid$|^math/]);
return schema;
}

Coloquei areLinksTrusted nas páginas que nós controlamos o conteúdo.

Legal, e deve valer a pena manter essa possibilidade mesmo se adotar a identificação automática dos links absolutos internos.

Os links de Fonte continuam sem o rel="nofollow" (acho que faz sentido, considerando que a Fonte seja usada da forma adequada, como "origem" do conteúdo).

Não temos como garantir essa "forma adequada", então acho que devemos seguir a mesma regra dos links no corpo dos conteúdos.

Futuramente podemos criar alguma heurística para não adicionar o plugin em conteúdos de usuários confiáveis

É uma boa! Outra opção é poder remover por conteúdo. Usuários confiáveis poderiam optar por usar TabCash para remover o nofollow do conteúdo. Ideia parecida pode se aplicar a adicionar a tag canonical.

ou usar a propriedade test para permitir domínios confiáveis (GitHub, por exemplo). Um exemplo de test que não colocará rel="nofollow" no link https://www.github.com/:

Legal, e acho que já podemos colocar o GitHub e o Curso.dev, e também o próprio TabNews, já que é outra forma de resolver o problema que citei sobre os links absolutos internos.

E também dá para adicionar a lógica de verificar os domínios lá na createRel. Tem que ver o trade-off, pois a ordem de verificação dentro do plugin é test && isAbsoluteUrl && rel.length. Também pesa na decisão a possibilidade de usar outras funções do rehype-extenral-links, pois ao não passar no test, nada será feito, enquanto definir rel=[] não impede as outras modificações que podem se aplicar aos links externos.


Será que a gente deveria permitir o uso de target="_blank" nos links criados pelos usuários? Eu acho que não, mas:

Se sim, acho que é bom adicionar também o noopener. É só passar no rel junto com o nofollow.

Se não, podemos remover o target através do sanitize. É só mudar isto

schema.attributes['*'] = schema.attributes['*'].filter((attr) => attr != 'className');

para isto

schema.attributes['*'] = schema.attributes['*'].filter((attr) => !['className', 'target'].includes(attr));

@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Jan 15, 2024

O plugin considera interno todos os links relativos, né? Só que está adicionando rel="nofollow" nos links de conteúdos criados pelos usuários com URLs absolutas do próprio TabNews. Será que isso não pode prejudicar o SEO?

Pelos testes que fiz, é isso mesmo. Vou experimentar o test verificando o .host como você mencionou.

Não temos como garantir essa "forma adequada", então acho que devemos seguir a mesma regra dos links no corpo dos conteúdos.

Ok, vou colocar o nofollow na fonte também.

Legal, e acho que já podemos colocar o GitHub e o Curso.dev, e também o próprio TabNews, já que é outra forma de resolver o problema que citei sobre os links absolutos internos.

👍 vou considerar esses domínios como confiáveis.

E também dá para adicionar a lógica de verificar os domínios lá na createRel. Tem que ver o trade-off, pois a ordem de verificação dentro do plugin é test && isAbsoluteUrl && rel.length. Também pesa na decisão a possibilidade de usar outras funções do rehype-external-links, pois ao não passar no test, nada será feito, enquanto definir rel=[] não impede as outras modificações que podem se aplicar aos links externos.

Pensando nisso que você falou e na sugestão #477, que provavelmente poderá ser implementada usando esse plugin, parece que o createRel fará mais sentido, mas vou avaliar melhor as opções.

Se sim, acho que é bom adicionar também o noopener. É só passar no rel junto com o nofollow.

Eu tinha pensado nisso quando li a documentação da biblioteca, mas não tinha colocado porque entendi pela documentação do MDN (a parte em destaque "Note") que hoje em dia os navegadores já fazem essa proteção por padrão. De toda forma, parece fazer sentido para caso alguém acesse com um navegador mais antigo ou sem essa proteção 👍

Acha necessário adicionar o noreferrer também?

Edit: para deixar claro, acho que não faz sentido deixar o criador do conteúdo escolher ter ou não o target="_blank", mas de toda forma faz sentido definirmos o uso do noopener e noreferrer por causa do issue #477.

Se não, podemos remover o target através do sanitize. É só mudar isto

Eu tenho a impressão que dá para tirar pelo rehype-external-links, mas não testei. Considerando que seja possível, você acha melhor deixar essa responsabilidade no sanitizer ou no plugin?

@Rafatcb Rafatcb added the pendente Aguardando ação do autor do Pull Request label Jan 15, 2024
@aprendendofelipe
Copy link
Collaborator

Acha necessário adicionar o noreferrer também?

Acho melhor não. Isso inviabilizaria outros sites medirem quando tráfego deles tem origem do TabNews.

para deixar claro, acho que não faz sentido deixar o criador do conteúdo escolher ter ou não o target="_blank", mas de toda forma faz sentido definirmos o uso do noopener e noreferrer por causa do issue #477.

Acho que não vale a pena implementar isso do #477. Gostei do artigo (When to use target=”_blank”) citado no README da rehype-extenral-links.

Eu tenho a impressão que dá para tirar pelo rehype-external-links, mas não testei. Considerando que seja possível, você acha melhor deixar essa responsabilidade no sanitizer ou no plugin?

Não acho que seja possível pelo rehype-external-links. Acho que só daria para adicionar, não remover. Mas mesmo que fosse possível, é melhor remover pelo sanitize, pois caso contrário não iria remover de links internos.

@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Jan 16, 2024

Acabei optando por não usar o webserver.host para comparar porque fiz o código de forma que possibilita comparação desconsiderando o subdomínio do link no href. Se for usar o webserver.host, precisará de um tratamento para remover o subdomínio da string, caso venha a ter um. Acha que faz sentido do jeito que está?

Deixei uma proteção para adicionar rel.push('noopener') caso voltem a aparecer links com target="_blank", mas se achar desnecessário, eu removo.

@Rafatcb Rafatcb removed the pendente Aguardando ação do autor do Pull Request label Jan 16, 2024
Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acabei optando por não usar o webserver.host para comparar porque fiz o código de forma que possibilita comparação desconsiderando o subdomínio do link no href. Se for usar o webserver.host, precisará de um tratamento para remover o subdomínio da string, caso venha a ter um. Acha que faz sentido do jeito que está?

Sim, faz sentido usar o TRUSTED_HOSTS, até porque isso pode depois ir para uma variável de ambiente, se for necessário, e não precisa estar vinculado ao webserver.host.

Mas fiz uma sugestão no código que também acaba usando o webserver.host. Veja se faz sentido.

Deixei uma proteção para adicionar rel.push('noopener') caso voltem a aparecer links com target="_blank", mas se achar desnecessário, eu removo.

Compensa deixar. 👍

tests/unit/interface/utils/trusted-domain.test.js Outdated Show resolved Hide resolved
pages/interface/utils/trusted-domain.js Outdated Show resolved Hide resolved
pages/interface/utils/trusted-domain.js Outdated Show resolved Hide resolved
tests/unit/interface/utils/trusted-domain.test.js Outdated Show resolved Hide resolved
@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Jan 17, 2024

@aprendendofelipe fiz as alterações, depois me diga se entendi algo errado ou se está tudo certo. Adicionei o webserver.host na lista de domínios confiáveis para conseguir testar caminhos relativos com o localhost.

Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depois me diga se entendi algo errado ou se está tudo certo.

Do que eu tinha falado, só faltou testar o protocolo relativo //. Não acha que compensa?

Adicionei o webserver.host na lista de domínios confiáveis para conseguir testar caminhos relativos com o localhost.

Show, então o que acha de usar o Set para remover o domínio duplicado? E também sugeri inverti a ordem dos confiáveis para o que acho que seriam os links mais comuns.

tests/unit/interface/utils/trusted-domain.test.js Outdated Show resolved Hide resolved
pages/interface/utils/trusted-domain.js Outdated Show resolved Hide resolved
…user generated content

Add `rel="nofollow"` to user-generated content links and also to publication sources, with a list of
domains considered trustworthy where this attribute will not be added.
 Remove `target="_blank"`
from links.
@Rafatcb
Copy link
Collaborator Author

Rafatcb commented Jan 18, 2024

Atualizei com suas recomendações, as alterações estão aqui.

@aprendendofelipe aprendendofelipe merged commit 4dd8894 into main Jan 18, 2024
7 checks passed
@aprendendofelipe aprendendofelipe deleted the use-rel-nofollow-on-links branch January 18, 2024 01:19
@aprendendofelipe
Copy link
Collaborator

Em produção 🚀🚀🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front Envolve modificações no frontend novo recurso Nova funcionalidade/recurso SEO Otimização do site para motores de busca
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Links nos artigos não tem o atributo rel="NOFOLLOW"
2 participants