Closed
Bug 402548
Opened 17 years ago
Closed 17 years ago
Autocomplete popup is not offset at the right position and width when page is zoomed
Categories
(Core :: XUL, defect, P3)
Tracking
()
RESOLVED
FIXED
People
(Reporter: martijn.martijn, Assigned: enndeakin)
References
()
Details
Attachments
(3 files, 4 obsolete files)
(deleted),
image/gif
|
Details | |
(deleted),
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
(not really a regression from bug 389628, but that bug makes it possible to encounter for users) To reproduce: - Go to http://www.google.nl/firefox and zoom in (or out) - Double-click in the input to open the autocomplete popup. Expected result: - Autocomplete popup should be aligned and have the same width as the text input of the page Actual result: - Autocomplete popup is a little off. It seems like the code doesn't make a zoom correction for the zoomed height and width of the page input.
Updated•17 years ago
|
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
Assignee | ||
Comment 1•17 years ago
|
||
This is because the width of the element is computed using getBoundingClientRect which returns the unscaled width, but we need to use the scaled width.
Assignee | ||
Comment 2•17 years ago
|
||
Martijn: the position of the popup looks ok to me, but only the width isn't right. If that's what you're seeing, then this is only a very minor bug.
Reporter | ||
Comment 3•17 years ago
|
||
This is what I get when I have the page zoomed in a lot. The popup is overlaying half of the input in that case.
Assignee | ||
Comment 6•17 years ago
|
||
This fixes the position of the autocomplete popup and also fixes bug 402551. It does not fix the width of the autocomplete popup. I will add some tests and then ask for review.
Assignee: jag → enndeakin
Status: NEW → ASSIGNED
Comment 8•17 years ago
|
||
Comment on attachment 288921 [details] [diff] [review] fixes autocomplete width Can't use getBrowser() in toolkit. This is also wrong for the Location bar and the Search bar if the content is zoomed, for instance.
Reporter | ||
Comment 9•17 years ago
|
||
Ok, thanks for the info. This is another approach.
Attachment #288921 -
Attachment is obsolete: true
Updated•17 years ago
|
Attachment #288986 -
Attachment is patch: true
Attachment #288986 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 10•17 years ago
|
||
Attachment #288881 -
Attachment is obsolete: true
Attachment #289009 -
Flags: superreview?(roc)
Attachment #289009 -
Flags: review?(roc)
I think a better way to write this would be to convert from appunits to device units via aPresContext->AppUnitsToDevUnits() and then back to CSS pixels or appunits as necessary. It's more clear.
Assignee | ||
Comment 12•17 years ago
|
||
(In reply to comment #11) > I think a better way to write this would be to convert from appunits to device > units via aPresContext->AppUnitsToDevUnits() and then back to CSS pixels or > appunits as necessary. It's more clear. > Can you explain what you mean here? What purpose would converting to devpixels and then back again serve?
Instead of calling GetPixelScale, which is a confusing API and actually unused currently, it would be more obvious what's going on if you convert parentRect to device pixels and then back to appunits in the appropriate coordinate system.
Assignee | ||
Comment 14•17 years ago
|
||
Attachment #289009 -
Attachment is obsolete: true
Attachment #289511 -
Flags: superreview?(roc)
Attachment #289511 -
Flags: review?(roc)
Attachment #289009 -
Flags: superreview?(roc)
Attachment #289009 -
Flags: review?(roc)
I still find this unclear. + // the anchor may be in a different document with a different scale, + // so adjust the rectangle to the scale of the popup. + parentRect.ScaleRoundOut(float(presContext->AppUnitsPerDevPixel()) / + aAnchorFrame->PresContext()->AppUnitsPerDevPixel()); We don't actually use the top/left which is good because it's kinda meaningless here. So lets make this parentSize and add a comment to make it clear that it's the size in this frame's appunits. I don't understand this screenpos code. Maybe it would be clearer if you start by converting mScreenPosX to unscaled dev pixels and then proceed from there.
Assignee | ||
Comment 16•17 years ago
|
||
Attachment #289511 -
Attachment is obsolete: true
Attachment #289673 -
Flags: superreview?(roc)
Attachment #289673 -
Flags: review?(roc)
Attachment #289511 -
Flags: superreview?(roc)
Attachment #289511 -
Flags: review?(roc)
Attachment #289673 -
Flags: superreview?(roc)
Attachment #289673 -
Flags: superreview+
Attachment #289673 -
Flags: review?(roc)
Attachment #289673 -
Flags: review+
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 17•17 years ago
|
||
Comment on attachment 288986 [details] [diff] [review] fixes autocomplete width, v2 Neil, do you think this is the correct way to solve this?
Attachment #288986 -
Flags: review?(enndeakin)
Assignee | ||
Comment 18•17 years ago
|
||
Comment on attachment 288986 [details] [diff] [review] fixes autocomplete width, v2 That's probably the best we can do for now. I wouldn't consider the popup width to be as much of a concern as the position patch in the bug though.
Attachment #288986 -
Flags: review?(enndeakin) → review+
Reporter | ||
Comment 19•17 years ago
|
||
Comment on attachment 288986 [details] [diff] [review] fixes autocomplete width, v2 Because this is toolkit, I assume this doesn't need sr.
Attachment #288986 -
Flags: approval1.9?
Reporter | ||
Comment 20•17 years ago
|
||
I filed bug 407912 for the width patch, since approval1.9 drivers don't 'see' bugs that are already fixed.
Reporter | ||
Updated•17 years ago
|
Attachment #288986 -
Flags: approval1.9?
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•