Toolbar menu shouldn't allow its text to be selected

VERIFIED FIXED in Firefox 21

Status

()

Firefox for Android
Reader View
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: ibarlow, Assigned: jwir3)

Tracking

unspecified
Firefox 19
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox21 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 662554 [details]
screenshot

Even though the menu is built in html, we shouldn't be allowing text selection to work on it. See screenshot
(Assignee)

Updated

6 years ago
Assignee: nobody → sjohnson
(Assignee)

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Updated

6 years ago
OS: All → Android
Hardware: All → ARM
(Assignee)

Comment 1

6 years ago
Created attachment 674012 [details] [diff] [review]
b792420
Attachment #674012 - Flags: review?(wjohnston)
Comment on attachment 674012 [details] [diff] [review]
b792420

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

::: mobile/android/chrome/content/aboutReader.html
@@ +26,5 @@
>      <li><a id="share-button" class="button share-button" href="#"></a></li>
>      <ul class="dropdown">
>        <li><a class="dropdown-toggle button style-button" href="#"></a></li>
>        <li class="dropdown-popup">
> +        <ul id="color-scheme-buttons" class="segmented-button" onmousedown="return false;"></ul>

What's this for? Remove it.

::: mobile/android/chrome/content/aboutReader.js
@@ +545,5 @@
>  
>        let item = doc.createElement("li");
>        let link = doc.createElement("a");
>        link.innerHTML = option.name;
> +      link.style.MozUserSelect = 'none';

We should just put this in the CSS for about:reader:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/themes/core/aboutReader.css#445

It looks like we could get rid of this anchor element entirely tbh, and just put the text in the li element, but that's a separate bug I guess (I'm also not sure why this is dynamically generated at all).
Attachment #674012 - Flags: review?(wjohnston) → review+
(Assignee)

Comment 3

6 years ago
(In reply to Wesley Johnston (:wesj) from comment #2)
> Comment on attachment 674012 [details] [diff] [review]
> What's this for? Remove it.

Ah yes, this was left over from when I was testing something else. I'll remove it.

> We should just put this in the CSS for about:reader:
Hm, so I tried putting the css rule in place. For some odd reason, it doesn't seem to have any effect, though. I tried placing -moz-user-select: none in the rules for the selector .segmented-button > li > a, with and without !important, as well as in a selector with just a, again with and without !important, and neither of them seem to have any effect. As a sanity check, I tried placing border: 1px solid red; in the same selector, and that takes effect as expected. 

Is it possible I might be missing something here? Do I need to do something else to get the rule to translate into aElement.style.MozUserSelect?

> It looks like we could get rid of this anchor element entirely tbh, and just
> put the text in the li element, but that's a separate bug I guess (I'm also
> not sure why this is dynamically generated at all).

Submitted bug 805569 for this.
(Assignee)

Comment 4

6 years ago
(In reply to Scott Johnson (:jwir3) from comment #3)
> Is it possible I might be missing something here? Do I need to do something
> else to get the rule to translate into aElement.style.MozUserSelect?

Ok, I found this:

This does not have any affect on actual selection operation. This doesn't have any effect on content loaded as chrome, except in textboxes.

from https://developer.mozilla.org/en-US/docs/CSS/user-select

So, it looks like this CSS property isn't set because this is chrome. Should I just keep it in the JS then, since I'm really only using it as a flag to determine whether or not the selection should be allowed?
(In reply to Scott Johnson (:jwir3) from comment #4)
> (In reply to Scott Johnson (:jwir3) from comment #3)
> > Is it possible I might be missing something here? Do I need to do something
> > else to get the rule to translate into aElement.style.MozUserSelect?
> 
> Ok, I found this:
> 
> This does not have any affect on actual selection operation. This doesn't
> have any effect on content loaded as chrome, except in textboxes.
> 
> from https://developer.mozilla.org/en-US/docs/CSS/user-select
> 
> So, it looks like this CSS property isn't set because this is chrome. Should
> I just keep it in the JS then, since I'm really only using it as a flag to
> determine whether or not the selection should be allowed?

Sounds like a good plan.
(Assignee)

Comment 6

6 years ago
Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f332ed25844e
Target Milestone: --- → Firefox 19

Comment 7

6 years ago
https://hg.mozilla.org/mozilla-central/rev/f332ed25844e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
This issue doesn't occur anymore on the latest Nightly. Closing bug as verified fixed on:

Firefox for Android
Version: 21.0a1 (2013-01-29)
Device: Galaxy R
OS: Android 2.3.4
status-firefox21: --- → verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.