1489874 - When submitting a content blocking issue report, pressing the left arrow (e.g. to fix a typo) exits the report and loses everything that was typed

Status: RESOLVED FIXED

(Firefox :: Site Identity, defect, P3)

Description

[Chris Conover]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:64.0) Gecko/20100101 Firefox/64.0
Build ID: 20180907100116

Steps to reproduce:

Mac: 10.13.6, FF 64.0a1

Go to a site where Content Tracking is enabled:
   https://www.politico.com/story/2018/09/09/pence-obama-trump-midterms-812655

Click on Content Tracking icon (i) -> Report a Problem, start typing, use back arrow.


Actual results:

Typing some text, then the back arrow (to correct a typo) clobbers entire form, forcing you to start again.  This is frustrating enough that I have given up on submitting site problems.


Expected results:

Back arrow should be left as back arrow, allowing you to edit text and not used as back navigation.

Comment 1

[Oana Arbuzov [:oanaarbuzov]]

I was able to reproduce the issue. Moving this issue to Tech Evangelism for further investigation.

[Tested with:]
Browser / Version: Firefox Nightly 64.0a1 (2018-09-11)
Operating System: Windows 10 Pro

@Mike can you have a look on this?

Comment 2

[Oana Arbuzov [:oanaarbuzov]]

Attachment: BackArrowDismiss.gif

Attached gif with the issue.

Comment 3

[Mike Taylor [:miketaylr]]

Thanks for the report! 

Johann, this seems like something on your end to tackle.

Comment 4

[Johann Hofmann [:johannh]]

Yeah!

Comment 6

[Tanvi Vyas[:tanvi]]

Would be great to get this into 64 if there are cycles.

Comment 8

[Johann Hofmann [:johannh]]

There is a deeper issue around not being able to move the cursor in text areas, namely bug 187083. I don't expect us to solve this.

We can, however, solve the issue of navigating with the arrow-keys in this panel. The code that is doing this lives here: https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/browser/components/customizableui/PanelMultiView.jsm#1566

In this context, this.node will refer to this element https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/browser/components/controlcenter/content/panel.inc.xul#317

It would be great if we could add an attribute to that element specifying that we don't want to enable navigation via back and forward keys, such as backforwardkeynavigation=false.

Then the above mentioned code would need to check that attribute and stop navigation.

Comment 9

[danielleleb12]

Having added the backforwardkeynavigation="false" on line 320 of the panel.inc.xul, is the best way to check the attribute by doing this:

      case "ArrowLeft":
      case "ArrowRight": {
        stop();
        if (this.node.getAttribute("backforwardkeynavigation") && 
            ((!this.window.RTL_UI && keyCode == "ArrowLeft") ||
            (this.window.RTL_UI && keyCode == "ArrowRight"))) {
          this.node.panelMultiView.goBack();
          break;
        }
        // If the current button is _not_ one that points to a subview, pressing
        // the arrow key shouldn't do anything.
        let button = this.selectedElement;
        if (!button || !button.classList.contains("subviewbutton-nav")) {
          break;
        }
        // Fall-through...
      }

Or should I be checking it a different way/somewhere else?

Comment 10

[Johann Hofmann [:johannh]]

Hey, I'm really sorry, it turns out that I think we will end up fixing this issue in bug 1522092. So there's probably no patch required here. It doesn't happen very often that two bugs collide, and I apologize for putting you onto this. Be sure that your effort was noted.

Can I just re-route you to work on bug 1501955 instead?

Thank you!

Comment 11

[danielleleb12]

Hey, no problem at all! It was interesting to take a look at the bug anyway.

I'll start working on the other bug instead.

Thanks!

Comment 12

[James Teh [:Jamie]]

Bug 1522092 did fix this, but it also caused other problems. I'm going to address this properly in bug 1477673.

Comment 13

[Johann Hofmann [:johannh]]

Let's keep this open for tracking then?

Comment 14

[Itiel]

Seems to be fixed after bug 1477673 has landed.