Closed Bug 858359 Opened 10 years ago Closed 10 years ago

Work - Remove Selectable context menu options on input and textareas when there is text and no selection

Categories

(Firefox for Metro Graveyard :: Browser, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: bbondy, Assigned: bbondy)

References

Details

Attachments

(2 files, 1 obsolete file)

The story in bug 844658 does not list select and select all. I'm not sure what select does but it seems like it does nothing. Select all has problems with the selection markers when using touch.

These should both be removed.
So the purpose of select is to add a selection marker if you long tap in a box with text.
Summary: Work - Remove Select and Select All from context menus on input and textareas → Work - Remove Selectable context menu options on input and textareas when there is text and no selection
Attached patch Patch v1.Splinter Review
I think I have to update a test or two, I'll be attaching another patch with that.
Attachment #734382 - Flags: review?(jmathies)
Attachment #734382 - Flags: review?(jmathies) → review+
Attached patch Patch v1 - tests (obsolete) — Splinter Review
Unfortunately even before my patches the tests for context menus fail with a timeout and some other failures, I think related to needing a yield (wait for text on clipboard) when copying text on the clipboard.   

I can get other browser chrome mochitests working, and the tests do run in the immersive environment so I'm running them correctly.

I can look into those failures later but I think this is the changes needed for this bug. If they don't fail for you maybe you could take this patch for a spin?
Attachment #734569 - Flags: review?(jmathies)
Comment on attachment 734569 [details] [diff] [review]
Patch v1 - tests

Review of attachment 734569 [details] [diff] [review]:
-----------------------------------------------------------------

I get the copy text failures too, I think the copy commands were changed to async a while back.

::: browser/metro/base/tests/mochitest/browser_context_menu_tests.js
@@ +184,5 @@
>      yield popupPromise;
>      ok(popupPromise && !(popupPromise instanceof Error), "promise error");
>  
>      let string = SpecialPowers.getClipboardData("text/unicode");
> +    ok(string === "hello", "copied selected text (hell0 === " + string + ")");

hell0 typo?

@@ +314,5 @@
>      purgeEventQueue();
>    }
>  });
>  
> +/*

why are all the image tests commented out?

@@ +398,5 @@
>      menuItem = document.getElementById("context-copy-image");
>      ok(menuItem, "menu item exists");
>      ok(!menuItem.hidden, "menu item visible");
>      popupPromise = waitForEvent(document, "popuphidden");
> +    EventUtils.synthesizeMouseAtCenter(menuItem, 10, 10, {}, win);

nix the coordinates here?
Hey Jim sorry I attached a patch I was investigating test failures with in my m-c dir instead of the one I wanted to apply in a different dir.
Attachment #734569 - Attachment is obsolete: true
Attachment #734569 - Flags: review?(jmathies)
Attachment #734578 - Flags: review?(jmathies)
Attachment #734578 - Flags: review?(jmathies) → review+
Comment on attachment 734382 [details] [diff] [review]
Patch v1.

no problem!
Attachment #734382 - Attachment is obsolete: true
Attachment #734382 - Flags: review+
OS: Windows 8 → Windows 8 Metro
Attachment #734382 - Attachment is obsolete: false
Attachment #734382 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/cee523a84f6b
https://hg.mozilla.org/mozilla-central/rev/f3e849b9ea6f
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.