No way to deselect image of image document after select all (Ctrl+A)

RESOLVED FIXED in mozilla15

Status

()

Core
Layout
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Alice0775 White, Assigned: Mats Palmgren (vacation - back in August))

Tracking

(Depends on: 1 bug, {regression})

11 Branch
mozilla15
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/8d1c74566a0b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120313 Firefox/13.0a1 ID:20120313090404

In Firefox11 and later,
No way to deselect image of image document after select all (Ctrl+A).

Reproducible: Always

Steps to Reproduce:
1. Start Firefox with Clean porofile
2. Open an image (ex. https://bugzilla.mozilla.org/skins/standard/index/help.png )
4. Ctrl + A
5. Try to deselect image

Actual Results:
 No way to deselect

Expected Results:
 Clicking border should deselect image

Regression range:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dca42848a1e2&tochange=2a0b56de8e9e
At least on mac, pressing the tab key twice deselects the image. Still clearly not an ideal situation.
I think this might have to do with the absolute positioning of the image, or at least that seems to me to be the only notable change from pre-376997.
Yeah, I noticed this too. Not sure what's causing it.

A simple wallpaper might be to use -moz-user-select:none to prevent the selection in the first place?
There isn't a direct way with the keyboard to deselect things that aren't images either, apart from tabbing away. We don't have a 'Deselect All' command.
(In reply to Neil Deakin from comment #4)
> There isn't a direct way with the keyboard to deselect things that aren't
> images either, apart from tabbing away. We don't have a 'Deselect All'
> command.

True, but try selecting some (or even all) text on the page, and then click somewhere that isn't on top of the selection. That clears the selection.
Actually, even clicking on top of the selection normally clears it.
OK, I see the image document has been changed to display in a different way. I didn't realize that this was talking about a regression.

Some debugging shows that the difference between position: absolute and not causes nsFrame::DrillDownToSelectionFrame to retreive a different frame when looking to see if the mouse is over an existing selection. The issue that the absolute placeholder frame for the image is considered to be empty so isn't considered part of the selection I guess.
(In reply to Justin Dolske [:Dolske] from comment #3)
> A simple wallpaper might be to use -moz-user-select:none to prevent the
> selection in the first place?
-moz-user-select:none didn't prevent Ctrl+A. Even worse, it prevented to deselect the selection!

Comment 9

5 years ago
Maybe the solution I posted on the 376997 bug page could help you.
https://bugzilla.mozilla.org/show_bug.cgi?id=376997
(In reply to Neil Deakin from comment #7)

> Some debugging shows that ...

Neil: should you own this? Or propose someone else?

Also, I just tried a quick workaround. If I use DOM Inspector to toss a text node into the document, deselection works properly. So perhaps we could just add a dummy space character? Doesn't fix the core problem, but gets it working properly.
Not me. Mats Palmgren or Masayuki might have more insight into the reasons behind comment 7.

The workaround sounds ok for image documents though.
CCing mats and masayuki per last comment.

Also, Jared and I were just talking about this, and I can reproduce this with a standalone HTML document so it's not just somehow unique to ImageDocuments.
Why don't you try disabling select all command on image document? It seems that the command doesn't make sense on such situation. If it's need to copy, I think copy command should be always enabled and copy the image without selection. It's easier for users.
That might be a workaround for the use case this bug was filed against. But I don't think that can be assumed for an arbitrary web page that hits this bug.
Created attachment 614379 [details] [diff] [review]
fix

This hack seems to be the root of the problem, it makes
nsFrameSelection::TakeFocus *fail* if the new node is the root node.
I don't see any reason for this hack since the same thing can be done
by setting up the Selection that way using script.

The hack was added 2000-03-21:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsSelection.cpp&rev=3.1&root=/cvsroot#1489
Created attachment 614561 [details] [diff] [review]
fix+test

The code change did pass regression tests on Try:
https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=a8fc86074929

Results for the added mochitest is pending:
https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=68918378eaaf
Assignee: nobody → matspal
Attachment #614379 - Attachment is obsolete: true
Attachment #614561 - Flags: review?(bzbarsky)
Attachment #614561 - Flags: feedback?(ehsan)
Comment on attachment 614561 [details] [diff] [review]
fix+test

r=me
Attachment #614561 - Flags: review?(bzbarsky) → review+
Attachment #614561 - Flags: feedback?(ehsan) → feedback+
https://hg.mozilla.org/integration/mozilla-inbound/rev/12cb1f643adb
Flags: in-testsuite+
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/12cb1f643adb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Depends on: 846943
You need to log in before you can comment on or make changes to this bug.