webcompat / web-bugs

A place to report bugs on websites.
https://webcompat.com
Mozilla Public License 2.0
743 stars 66 forks source link

dev.to - The menu options are not displayed #55782

Closed RSuraj closed 4 years ago

RSuraj commented 4 years ago

URL: https://dev.to/snappydevlpr/vs-code-web-dev-2f9k

Browser / Version: Firefox Mobile 68.0 Operating System: Android Tested Another Browser: Yes Chrome

Problem type: Site is not usable Description: Buttons or links not working Steps to Reproduce: The menu like button on the bottom bar (right end) doesn't work. Clicking doesn't yield any results.

View the screenshot Screenshot
Browser Configuration
  • None

From webcompat.com with ❤️

cipriansv commented 4 years ago

Thanks for the report, @RSuraj.

I was indeed able to reproduce the issue. After logging in and tapping the menu button from the bottom-right the menu options were not revealed.

Tested with: Browser / Version: Firefox Nightly 200706(🦎 80.0a1-20200702094606), Chrome Mobile 83.0.4103.106 Operating System: OnePlus6 (Android 10) - 1080 x 2280 pixels (~402 ppi pixel density)

This is the web page displayed in Firefox Preview Nightly after tapping the menu button:

image

And this is the web page displayed in Chrome:

image

Moving the issue to needsdiagnosis.

miketaylr commented 4 years ago

So the 3 dot menu is an html <button> with a nested SVG element. Then there's some custom elements as siblings to the button that have the menu content.

<div class="align-center m:relative">
      <button id="article-show-more-button" class="dropbtn crayons-btn crayons-btn--ghost crayons-btn--icon-rounded" aria-label="Toggle dropdown menu">
        <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" role="img" aria-labelledby="a8ax8qtjp4l53dffkpddojwwywa7ytcg" class="dropdown-icon crayons-icon"><title id="a8ax8qtjp4l53dffkpddojwwywa7ytcg">More...</title><path fill-rule="evenodd" clip-rule="evenodd" d="M7 12a2 2 0 11-4 0 2 2 0 014 0zm7 0a2 2 0 11-4 0 2 2 0 014 0zm5 2a2 2 0 100-4 2 2 0 000 4z"></path></svg>
      </button>
      <div class="crayons-dropdown p-1 z-30 right-1 left-1 s:left-auto bottom-100 m:bottom-0 m:right-auto m:left-100 fs-base">
        <clipboard-copy for="article-copy-link-input" aria-live="polite" aria-controls="article-copy-link-announcer" class="dropdown-link-row">
          <div class="flex items-center">
            <input type="text" id="article-copy-link-input" value="https://dev.to/snappydevlpr/vs-code-web-dev-2f9k" class="crayons-textfield" readonly="">
            <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" id="article-copy-icon" role="img" aria-labelledby="aq79yr2c2x55et6logbe1guu6jp0hweq" class="crayons-icon mx-2 shrink-0"><title id="aq79yr2c2x55et6logbe1guu6jp0hweq">Notifications</title>
    <path d="M7 6V3a1 1 0 011-1h12a1 1 0 011 1v14a1 1 0 01-1 1h-3v3c0 .552-.45 1-1.007 1H4.007A1 1 0 013 21l.003-14c0-.552.45-1 1.007-1H7zm2 0h8v10h2V4H9v2zm-2 5v2h6v-2H7zm0 4v2h6v-2H7z"></path>
</svg>

          </div>
          <div id="article-copy-link-announcer" role="alert" class="crayons-notice crayons-notice--success my-2 p-1" hidden="">Copied to Clipboard</div>
        </clipboard-copy>
        <web-share-wrapper shareurl="https://dev.to/snappydevlpr/vs-code-web-dev-2f9k" sharetext="VS Code &amp; Web Dev" template="web-share-button">
          <a target="_blank" class="dropdown-link-row crayons-link crayons-link--block" rel="noopener" href="https://twitter.com/intent/tweet?text=&quot;VS%20Code%20%26%20Web%20Dev&quot; by @amsuare %23DEVCommunity https://dev.to/snappydevlpr/vs-code-web-dev-2f9k">
            Share to Twitter
          </a>
          <a target="_blank" class="dropdown-link-row crayons-link crayons-link--block" rel="noopener" href="https://www.linkedin.com/shareArticle?mini=true&amp;url=https://dev.to/snappydevlpr/vs-code-web-dev-2f9k&amp;title=VS Code &amp; Web Dev&amp;summary=There are many different text editors out there but by far one of my favorites is VS Code. It's an in...&amp;source=DEV">
            Share to LinkedIn
          </a>
          <a target="_blank" class="dropdown-link-row crayons-link crayons-link--block" rel="noopener" href="https://www.reddit.com/submit?url=https://dev.to/snappydevlpr/vs-code-web-dev-2f9k&amp;title=VS Code &amp; Web Dev">
            Share to Reddit
          </a>
          <a target="_blank" class="dropdown-link-row crayons-link crayons-link--block" rel="noopener" href="https://news.ycombinator.com/submitlink?u=https://dev.to/snappydevlpr/vs-code-web-dev-2f9k&amp;t=VS Code &amp; Web Dev">
            Share to Hacker News
          </a>
          <a target="_blank" class="dropdown-link-row crayons-link crayons-link--block" rel="noopener" href="https://www.facebook.com/sharer.php?u=https://dev.to/snappydevlpr/vs-code-web-dev-2f9k">
            Share to Facebook
          </a>
        </web-share-wrapper>
        <template id="web-share-button">
          <a href="#" class="dropdown-link-row crayons-link crayons-link--block">Share Post</a>
        </template>
        <a href="/report-abuse" class="dropdown-link-row crayons-link crayons-link--block">Report Abuse</a>
      </div>
    </div>

By default the .crayons-dropdown div is display:none. If I manually toggle that, it displays as expected.

Still not sure why the click event isn't reacting properly. It seems like all the events are instrument with something like Sentry, so the debugger is sort of useless.

I did make https://miketaylr.com/bzla/svg-button-click.html to make sure there wasn't anything weird there (and there is a difference between Chrome and Firefox in terms of e.target, but they're both part of the SVG element, so I don't think that's it).

I also see a reference to something called InstantClick in base.js, and it seems like something just like FastClick... http://instantclick.io/ possibly the issue?

miketaylr commented 4 years ago

Maybe a red-herring. Maybe not.

function toggleEllipsisMenu(e) {
var t = getMenu(e.target);
hideAllEllipsisMenusExcept(t),
t.classList.contains('block') ? t.classList.remove('block')  : t.classList.add('block')
}

I can't ever get that to trip a breakpoint. Which maybe makes sense, if the click handler is never set:

function initializeEllipsisMenuToggle() {
  var buttons = document.getElementsByClassName(
    'js-dashboard-row-more-trigger',
  );

  for (var i = 0; i < buttons.length; i += 1) {
    buttons[i].addEventListener('click', toggleEllipsisMenu);
  }

  // Hide ellipsis menus when you click outside of the ellipsis menu parent div
  const body = document.body;
  if (body) {
    body.addEventListener('click', hideEllipsisMenus);
  }
}

In theory, it loops through all the buttons that descend from .js-dashboard-row-more-trigger, but I don't see that DOM element. So it never gets set?

But, I don't see it in Chrome Mobile either, and it works there.

miketaylr commented 4 years ago

Neat, the site is open source: https://github.com/forem/forem/blob/8c83aa336570e9f9085dcf4869270691ce131448/app/assets/javascripts/initializers/initializeEllipsisMenu.js#L144

https://github.com/forem/forem/blob/9f9236164cad2d1a8cc30784ab91896d39c87ad4/app/views/dashboards/_dashboard_article_row.html.erb#L93 is what I would expect (note the js-dashboard-row-more-trigger element). Maybe there's some code that removes the class?

Actually, I just tested in Fennec and the menu works as expected. So this is looking like a Fenix / GeckoView bug (even if I don't understand it yet....).

miketaylr commented 4 years ago

If I add the following rule to the page, it works as expected:

button > * {
  pointer-events: none;
}

So this suggests event bubbling isn't working as expected, when compared to Fennec.

miketaylr commented 4 years ago

Exploring https://miketaylr.com/bzla/svg-button-click.html again:

browser logged object (event.target)
Fenix [object SVGPathElement]
Fennec [object HTMLButtonElement]
Chrome Mobile [object HTMLButtonElement]
Chrome Desktop [object SVGSVGElement]
Firefox Desktop [object SVGSVGElement]
miketaylr commented 4 years ago

OK, I think I get why this fails now (took me long enough).

  1. the click handler for the ellipsis menu happens here:
function addDropdownListener(dropdown) {
    dropdown.addEventListener('click', dropdownFunction);
  }

  setTimeout(function addListeners() {
    getAllByClassName('dropbtn').forEach(addDropdownListener);
  }, 100);

Here's dropdownFunction:

  function dropdownFunction(e) {
    const button = e.currentTarget;
    const dropdownContent = button.parentElement.getElementsByClassName(
      'crayons-dropdown',
    )[0];

    if (!dropdownContent) {
      return;
    }

    finalizeAbuseReportLink(
      dropdownContent.querySelector('.report-abuse-link-wrapper'),
    );

    if (dropdownContent.classList.contains('block')) {
      dropdownContent.classList.remove('block');
      removeClickListener();
      removeCopyListener();
      hideAnnouncer();
    } else {
      removeAllShowing();
      dropdownContent.classList.add('block');
      const clipboardCopyElement = document.getElementsByTagName(
        'clipboard-copy',
      )[0];

      document.addEventListener('click', outsideClickListener);
      if (clipboardCopyElement) {
        clipboardCopyElement.addEventListener('click', copyText);
      }
    }
  }

Basically this fails at line 1: const button = e.currentTarget;. It assumes it has a reference to the <button>, but in Fenix, it has a reference to the child SVG content. So when it tries to find dropdownContent, it's undefined because the [0]th item of an empty HTMLCollection is... undefined.

So it hits the special early return:

if (!dropdownContent) {
      return;
    }

This code would also fail, because the parent <button> element is the one with these classes:

 function shouldCloseDropdown(event) {
    return !(
      event.target.matches('.dropdown-icon') ||
      event.target.matches('.dropbtn') ||
      event.target.matches('clipboard-copy') ||
      event.target.matches('clipboard-copy input') ||
      event.target.matches('clipboard-copy svg') ||
      event.target.parentElement.classList.contains('dropdown-link-row')
    );
  }

So that's why the pointer:events thing fixes the site -- doing that causes event.target to be the HTMLButton.

miketaylr commented 4 years ago

A relatively safe CSS injection could look like this:

#article-show-more-button > * {
  pointer-events: none
}

@ksy36 do you think we should do it while we wait on a bug fix? dev.to is a pretty popular site.

miketaylr commented 4 years ago

Another interesting thing: you can actually get one of SVGSVGElement, SVGPathElement or HTMLButtonElement on desktop (even on the real site), depending on how carefully you click. If you end up with SVGPathElement, the menu doesn't open -- so it's kind of a Fenix bug and kind of a site bug.

miketaylr commented 4 years ago

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1654934 to track the core issue. Leaving open until @ksy36 responds about the site patch.

ksy36 commented 4 years ago

yeah, sounds good, I've filed a bug https://bugzilla.mozilla.org/show_bug.cgi?id=1655049

softvision-oana-arbuzov commented 3 years ago

The issue seems to be fixed and works regardless if the Interventions is enabled or not.

Tested with: Browser / Version: Firefox Nightly 90.0a1 (🦎 90.0a1-20210510093555) Operating System: Google Pixel 5 (Android 11) - 1080 x 2340 pixels, 19.5:9 ratio (~432 ppi density)