Closed
Bug 735641
Opened 13 years ago
Closed 13 years ago
No way to deselect image of image document after select all (Ctrl+A)
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: alice0775, Assigned: MatsPalmgren_bugz)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
bzbarsky
:
review+
ehsan.akhgari
:
feedback+
|
Details | Diff | Splinter Review |
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
Comment 1•13 years ago
|
||
At least on mac, pressing the tab key twice deselects the image. Still clearly not an ideal situation.
Comment 2•13 years ago
|
||
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.
Comment 3•13 years ago
|
||
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?
Comment 4•13 years ago
|
||
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.
Comment 5•13 years ago
|
||
(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.
Comment 6•13 years ago
|
||
Actually, even clicking on top of the selection normally clears it.
Comment 7•13 years ago
|
||
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.
Comment 8•13 years ago
|
||
(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•13 years ago
|
||
Maybe the solution I posted on the 376997 bug page could help you.
https://bugzilla.mozilla.org/show_bug.cgi?id=376997
Comment 10•13 years ago
|
||
(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.
Comment 11•13 years ago
|
||
Not me. Mats Palmgren or Masayuki might have more insight into the reasons behind comment 7.
The workaround sounds ok for image documents though.
Comment 12•13 years ago
|
||
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.
Comment 13•13 years ago
|
||
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.
Comment 14•13 years ago
|
||
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.
Assignee | ||
Comment 15•13 years ago
|
||
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
Assignee | ||
Comment 16•13 years ago
|
||
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 17•13 years ago
|
||
Comment on attachment 614561 [details] [diff] [review]
fix+test
r=me
Attachment #614561 -
Flags: review?(bzbarsky) → review+
Updated•13 years ago
|
Attachment #614561 -
Flags: feedback?(ehsan) → feedback+
Assignee | ||
Comment 18•13 years ago
|
||
Flags: in-testsuite+
Target Milestone: --- → mozilla15
Comment 19•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•