[Bug] PR Review textfield can be broken

[delvh]

• Gitea version (or commit ref): 1.13.2
• Git version: 2.17.1
• Operating system: Ubuntu 18.04
• Can you reproduce the bug at https://try.gitea.io:
  • Nondeterministic (can only be reproduced with probability ~6% on 1.13.2)

Description

Sometimes (I have no idea under which circumstances - just affects some PRs, and most not…), it can happen that the popup generated by clicking the green (blue) Review Button gets displayed twice (or even more times, again, the number seems nondeterministic).
I noticed this issue already in 1.11 (without filing a bug report), where once the textfield to write a comment for the PR review got displayed 12 times (I have video proof of that!) instead of once. I've noticed this issue much more often in 1.11, this may however be due to the differing amount of PRs I reviewed under 1.11 vs. 1.13.2 until now.
As I noticed that issue again in 1.13.2, it still seems to be present.

Screenshots

(The textarea in this screenshot appears squished because I had the browser console open while taking it)

[delvh]

For troubleshouting this issue I have the following thought:
Since both the happening of the bug and the number of displayed text areas seems non-deterministic indepedent of each other,
it is most likely that not a single bug but two bugs acting together are responsible for creating this issue to its current extend.

[zeripath]

it's likely a race or a javascript error.

Is there any information in the javascript console?

[delvh]

Sorry, to tell you whether there is anything getting printed out in the console, I would first need to reproduce the bug, which given the undeterministic nature of this bug is not easy.

[zeripath]

Yeah I can imagine...

Next time I'm in that area of JavaScript I'll take a look but I think it's all a bit magic and I personally don't understand it all too well. (We really need to comment better.)

[delvh]

Funny thing: After a lot of reloading from the PR where the screenshot was taken as well, I've been able to reproduce it somehow .
I can add a few observations now:

1. There seem to be PRs where this issue is totally unreproducible
2. Your assumption about race conditions seems to be correct: I'm 90% certain that I saw only one textfield for a split second before the second was added
3. No console output gets generated
4. This is the code that gets generated more than once:

<!--<textarea name="content" tabindex="0" rows="2" placeholder="Review comment" style="display: none;">
</textarea> (is uncommented but greyed out in developer console)-->
<div class="editor-toolbar">
  <a title="Bold (Ctrl-B)" tabindex="-1" class="fa fa-bold">
  </a>
  <a title="Italic (Ctrl-I)" tabindex="-1" class="fa fa-italic">
  </a>
  <a title="Strikethrough" tabindex="-1" class="fa fa-strikethrough">
  </a>
  <i class="separator">|
  </i>
  <a title="Big Heading" tabindex="-1" class="fa fa-header fa-header-x fa-header-1">
  </a>
  <a title="Medium Heading" tabindex="-1" class="fa fa-header fa-header-x fa-header-2">
  </a>
  <a title="Small Heading" tabindex="-1" class="fa fa-header fa-header-x fa-header-3">
  </a>
  <a title="Bigger Heading (Shift-Ctrl-H)" tabindex="-1" class="fa fa-header fa-header-x fa-header-bigger">
  </a>
  <a title="Smaller Heading (Ctrl-H)" tabindex="-1" class="fa fa-header fa-header-x fa-header-smaller">
  </a>
  <i class="separator">|
  </i>
  <a title="Code (Ctrl-Alt-C)" tabindex="-1" class="fa fa-code">
  </a>
  <a title="Quote (Ctrl-')" tabindex="-1" class="fa fa-quote-left">
  </a>
  <i class="separator">|
  </i>
  <a title="Add Checkbox (empty)" tabindex="-1" class="fa fa-square-o">
  </a>
  <a title="Add Checkbox (checked)" tabindex="-1" class="fa fa-check-square-o">
  </a>
  <i class="separator">|
  </i>
  <a title="Generic List (Ctrl-L)" tabindex="-1" class="fa fa-list-ul">
  </a>
  <a title="Numbered List (Ctrl-Alt-L)" tabindex="-1" class="fa fa-list-ol">
  </a>
  <i class="separator">|
  </i>
  <a title="Create Link (Ctrl-K)" tabindex="-1" class="fa fa-link">
  </a>
  <a title="Insert Image (Ctrl-Alt-I)" tabindex="-1" class="fa fa-picture-o">
  </a>
  <a title="Insert Table" tabindex="-1" class="fa fa-table">
  </a>
  <a title="Insert Horizontal Line" tabindex="-1" class="fa fa-minus">
  </a>
  <i class="separator">|
  </i>
  <a title="Clean block (Ctrl-E)" tabindex="-1" class="fa fa-eraser fa-clean-block">
  </a>
  <i class="separator">|
  </i>
  <a title="Revert to simple textarea" tabindex="-1" class="fa fa-file">
  </a>
</div>
<div class="CodeMirror cm-s-paper CodeMirror-wrap CodeMirror-empty">
  <div style="overflow: hidden; position: relative; width: 3px; height: 0px; top: 14px; left: 15px;">
    <textarea style="position: absolute; padding: 0px; width: 1px; height: 1em; outline: currentcolor none medium;" autocorrect="off" autocapitalize="none" spellcheck="false" tabindex="0" class="js-quick-submit" data-tribute="true" wrap="off">
    </textarea>
  </div>
  <div class="CodeMirror-vscrollbar" cm-not-content="true">
    <div style="min-width: 1px; height: 0px;">
    </div>
  </div>
  <div class="CodeMirror-hscrollbar" cm-not-content="true">
    <div style="height: 100%; min-height: 1px; width: 0px;">
    </div>
  </div>
  <div class="CodeMirror-scrollbar-filler" cm-not-content="true">
  </div>
  <div class="CodeMirror-gutter-filler" cm-not-content="true">
  </div>
  <div class="CodeMirror-scroll" tabindex="-1" draggable="true">
    <div class="CodeMirror-sizer" style="margin-left: 0px; margin-bottom: -6px; border-right-width: 24px; min-height: 25px; padding-right: 0px; padding-bottom: 0px;">
      <div style="position: relative; top: 0px;">
        <div class="CodeMirror-lines">
          <div style="position: relative; outline: currentcolor none medium;">
            <pre style="height: 0px; overflow: visible;" class="CodeMirror-placeholder">Review comment
</pre>
            <div class="CodeMirror-measure">
              <span>
                <span>​
                </span>x
              </span>
            </div>
            <div class="CodeMirror-measure">
            </div>
            <div style="position: relative; z-index: 1;">
            </div>
            <div class="CodeMirror-cursors" style="">
              <div class="CodeMirror-cursor" style="left: 4px; top: 0px; height: 16.5px;">&nbsp;
              </div>
            </div>
            <div class="CodeMirror-code">
              <pre class=" CodeMirror-line ">
<span>
<span cm-text="">​
</span>
</span>
</pre>
            </div>
          </div>
        </div>
      </div>
    </div>
    <div style="position: absolute; height: 24px; width: 1px; top: 25px; border-bottom: 0px solid transparent;">
    </div>
    <div class="CodeMirror-gutters" style="display: none; height: 124px;">
    </div>
  </div>
</div>
<div class="editor-preview-side">
</div>
<div class="editor-statusbar">
  <span class="autosave">
  </span>
  <span class="lines">1
  </span>
  <span class="words">0
  </span>
  <span class="cursor">0:0
  </span>
</div>
<div class="editor-toolbar">
  <a title="Bold (Ctrl-B)" tabindex="-1" class="fa fa-bold">
  </a>
  <a title="Italic (Ctrl-I)" tabindex="-1" class="fa fa-italic">
  </a>
  <a title="Strikethrough" tabindex="-1" class="fa fa-strikethrough">
  </a>
  <i class="separator">|
  </i>
  <a title="Big Heading" tabindex="-1" class="fa fa-header fa-header-x fa-header-1">
  </a>
  <a title="Medium Heading" tabindex="-1" class="fa fa-header fa-header-x fa-header-2">
  </a>
  <a title="Small Heading" tabindex="-1" class="fa fa-header fa-header-x fa-header-3">
  </a>
  <a title="Bigger Heading (Shift-Ctrl-H)" tabindex="-1" class="fa fa-header fa-header-x fa-header-bigger">
  </a>
  <a title="Smaller Heading (Ctrl-H)" tabindex="-1" class="fa fa-header fa-header-x fa-header-smaller">
  </a>
  <i class="separator">|
  </i>
  <a title="Code (Ctrl-Alt-C)" tabindex="-1" class="fa fa-code">
  </a>
  <a title="Quote (Ctrl-')" tabindex="-1" class="fa fa-quote-left">
  </a>
  <i class="separator">|
  </i>
  <a title="Add Checkbox (empty)" tabindex="-1" class="fa fa-square-o">
  </a>
  <a title="Add Checkbox (checked)" tabindex="-1" class="fa fa-check-square-o">
  </a>
  <i class="separator">|
  </i>
  <a title="Generic List (Ctrl-L)" tabindex="-1" class="fa fa-list-ul">
  </a>
  <a title="Numbered List (Ctrl-Alt-L)" tabindex="-1" class="fa fa-list-ol">
  </a>
  <i class="separator">|
  </i>
  <a title="Create Link (Ctrl-K)" tabindex="-1" class="fa fa-link">
  </a>
  <a title="Insert Image (Ctrl-Alt-I)" tabindex="-1" class="fa fa-picture-o">
  </a>
  <a title="Insert Table" tabindex="-1" class="fa fa-table">
  </a>
  <a title="Insert Horizontal Line" tabindex="-1" class="fa fa-minus">
  </a>
  <i class="separator">|
  </i>
  <a title="Clean block (Ctrl-E)" tabindex="-1" class="fa fa-eraser fa-clean-block">
  </a>
  <i class="separator">|
  </i>
  <a title="Revert to simple textarea" tabindex="-1" class="fa fa-file">
  </a>
</div>
<div class="CodeMirror cm-s-paper CodeMirror-wrap CodeMirror-empty">
  <div style="overflow: hidden; position: relative; width: 3px; height: 0px; top: 14px; left: 15px;">
    <textarea style="position: absolute; padding: 0px; width: 1px; height: 1em; outline: currentcolor none medium;" autocorrect="off" autocapitalize="none" spellcheck="false" tabindex="0" class="js-quick-submit" data-tribute="true" wrap="off">
    </textarea>
  </div>
  <div class="CodeMirror-vscrollbar" cm-not-content="true">
    <div style="min-width: 1px; height: 0px;">
    </div>
  </div>
  <div class="CodeMirror-hscrollbar" cm-not-content="true">
    <div style="height: 100%; min-height: 1px; width: 0px;">
    </div>
  </div>
  <div class="CodeMirror-scrollbar-filler" cm-not-content="true">
  </div>
  <div class="CodeMirror-gutter-filler" cm-not-content="true">
  </div>
  <div class="CodeMirror-scroll" tabindex="-1" draggable="true">
    <div class="CodeMirror-sizer" style="margin-left: 0px; margin-bottom: -6px; border-right-width: 24px; min-height: 25px; padding-right: 0px; padding-bottom: 0px;">
      <div style="position: relative; top: 0px;">
        <div class="CodeMirror-lines">
          <div style="position: relative; outline: currentcolor none medium;">
            <pre style="height: 0px; overflow: visible;" class="CodeMirror-placeholder">Review comment
</pre>
            <div class="CodeMirror-measure">
              <pre>
<span>xxxxxxxxxx
</span>
</pre>
            </div>
            <div class="CodeMirror-measure">
            </div>
            <div style="position: relative; z-index: 1;">
            </div>
            <div class="CodeMirror-cursors">
              <div class="CodeMirror-cursor" style="left: 4px; top: 0px; height: 16.5px;">&nbsp;
              </div>
            </div>
            <div class="CodeMirror-code">
              <pre class=" CodeMirror-line ">
<span>
<span cm-text="">​
</span>
</span>
</pre>
            </div>
          </div>
        </div>
      </div>
    </div>
    <div style="position: absolute; height: 24px; width: 1px; top: 25px; border-bottom: 0px solid transparent;">
    </div>
    <div class="CodeMirror-gutters" style="display: none; height: 124px;">
    </div>
  </div>
</div>
<div class="editor-preview-side">
</div>
<div class="editor-statusbar">
  <span class="autosave">
  </span>
  <span class="lines">1
  </span>
  <span class="words">0
  </span>
  <span class="cursor">0:0
  </span>
</div>
		

[delvh]

Appears to be a duplicate of/ too similar to #12573, hence closing this issue.