46555 - "Select All" is enabled even when text field is empty

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Eli Goldberg]

* TITLE/SUMMARY
"Select All" is enabled even when text field is empty

* STEPS TO REPRODUCE
0) Launch Seamonkey
1) Clear the content of the URL bar
2) Open the "Edit" menu

* RESULT
 - What happened

"Select All" is enabled.

 - What was expected 

"Select All" should be disabled unless the URL bar contains content. (Unable to 
check via context menus)

Note that this also occurs in form text fields, both using the main application 
menu and using context menus.

* REGRESSION

 - Occurs On
        Mac OS/Win32/Linux Seamonkey (200072608 optimized build)		

 - Doesn't Occur On
        Communicator 4.7.4 RTM (Mac OS)




* CONFIGURATIONS TESTED

- [Mac] Power Mac G4 (450 Mhz), 256 MB RAM (VM off), 1024x768 (Thousands of 
Colors), Mac OS 9.0

- [Win32] Vectra VL (266 MHz P2), 96 MB RAM, 800x600 (True Color), NT 4.0 SP5.

- [Linux] Vectra VL (266 MHz P2), 96 MB RAM. Red Hat Linux 6.0 (GNOME).

Comment 1

[rubydoo123]

setting to m19

Comment 2

[rubydoo123]

moving to future per review with bij and beppe

Comment 3

[rubydoo123]

removing myself from the cc list

Comment 4

[Michal Bukovjan]

Attachment: Proposed patch

This patch fixes the problem.
It exposes an editor buglet, though:
In an HTML textarea (and only in there, not in edits) when I type a letter into
an empty textarea, then delete it (Backspace key), I can still Select all.
If I type a letter, do Select All, then delete it (Del key), the command is
correctly disabled.
Probably should be filed as another bug.

Comment 5

[Simon Fraser [no longer active]]

Over to Joe.

Comment 6

[Joe Francis]

Comment on attachment 93039 [details] [diff] [review]
Proposed patch

r=jfrancis

Comment 7

[Joe Francis]

simon, sr?

Comment 8

[Simon Fraser [no longer active]]

+      *outCmdEnabled = !(*outCmdEnabled);

That's a bit yucky. How about:

PRBool docIsEmpty;
nsresult res = aEditor->GetDocumentIsEmpty(&docIsEmpty);
...
*outCmdEnabled = !docIsEmpty;

Comment 9

[Joe Francis]

differentiating bug severity of my most critical bugs vai abuse of milestone field
M2: severe
M1: very severe and/or fix in hand

Comment 11

[Graeme McCutcheon [:graememcc]]

> It exposes an editor buglet, though:
> In an HTML textarea (and only in there, not in edits) when I type a letter into
> an empty textarea, then delete it (Backspace key), I can still Select all.
> If I type a letter, do Select All, then delete it (Del key), the command is
> correctly disabled.
> Probably should be filed as another bug.

Bug 483651

Comment 12

[Graeme McCutcheon [:graememcc]]

Attachment: Updated patch, with test

Comment 13

[(no longer active)]

Comment on attachment 554532 [details] [diff] [review]
Updated patch, with test

Looks good!

Have you also run this through the try server?

Comment 14

[Graeme McCutcheon [:graememcc]]

Hm, this isn't ready to be landed yet.

On try, browser/base/content/tests/test_contextmenu.html failed, due to it testing for select all being enabled for an empty text input. Changing the test to reflect that it will be disabled, and the same check fails on Mac. Fun times.

Looking into this...

Comment 15

[Graeme McCutcheon [:graememcc]]

Attachment: Update to test_contextmenu.html

We'll need to land this as well when bug 682618 is fixed.

Comment 16

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4297a90d3bea

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4297a90d3bea