The default bug view has changed. See this FAQ.

Infobar disappears when node fills top and bottom of screen during page zoom in highlighter

RESOLVED FIXED in Firefox 12

Status

()

Firefox
Developer Tools
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: rc, Assigned: paul)

Tracking

unspecified
Firefox 12
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
STR:

1. Open Highlighter
2. Select a large node on the screen.
3. Hit ctrl/cmd-+ several times so the node reaches both top and bottom of the window

Expected results:

Infobar should move to the top of the available window space inside the node.

Actual:

Infobar disappears offscreen.
(Assignee)

Updated

6 years ago
Assignee: nobody → paul
(Assignee)

Comment 1

6 years ago
I can't reproduce. Can you give me an example?
(Assignee)

Comment 2

6 years ago
That might be fixed once bug 690068 get fixed.
(Assignee)

Updated

6 years ago
Blocks: 663830

Comment 3

6 years ago
I can reproduce this with a bugzilla body element, but only zooming out, not zooming in.
(Reporter)

Comment 4

6 years ago
Ditto, replace Cmd-+ in C#0 with Cmd-- and you can see this bug in action.

Dave suggested the reason might be that we're doing our bounds checking in the pre-scaled coordinate space.
(Assignee)

Comment 5

6 years ago
Bug 690068 hasn't fixed this problem.

Comment 6

6 years ago
We're doing developer tool prioritization, filter on 'brontozaur'
to ignore the spam.
Priority: -- → P2
(Assignee)

Comment 7

5 years ago
Created attachment 579683 [details] [diff] [review]
patch v1
(Assignee)

Updated

5 years ago
Attachment #579683 - Flags: review?(rcampbell)
(Assignee)

Updated

5 years ago
Blocks: 694954
(Reporter)

Updated

5 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 8

5 years ago
Comment on attachment 579683 [details] [diff] [review]
patch v1

       case "resize":
+        this.getZoomFactor(true);

are there any other cases where you'll want to send true to getZoomFactor?

Without that, we could do away with the cached value (this._zoom) and just call the lookup directly. It should be pretty fast and we'd be able to use a real getter.

I don't feel strongly about it though and this looks like it'll do the job nicely, so...
Attachment #579683 - Flags: review?(rcampbell) → review+
(Reporter)

Updated

5 years ago
Whiteboard: [needs-test]
(Assignee)

Comment 9

5 years ago
Created attachment 582830 [details] [diff] [review]
patch v1.1

better?
(Assignee)

Updated

5 years ago
Attachment #582830 - Flags: review?(rcampbell)
(Reporter)

Comment 10

5 years ago
Comment on attachment 582830 [details] [diff] [review]
patch v1.1

you can probably reuse the test in browser_inspector_highlighter.js which already does some zoom calculation. Making it call your computeZoomFactor method will get it tested.

r+ with changes to the test to make that happen.
Attachment #582830 - Flags: review?(rcampbell) → review+
(Assignee)

Comment 11

5 years ago
Created attachment 582839 [details] [diff] [review]
patch v1.2

with test
(Assignee)

Updated

5 years ago
Attachment #582830 - Attachment is obsolete: true
(Assignee)

Comment 12

5 years ago
Thx for the r+. Updated the test as you asked.
Whiteboard: [needs-test] → [land-in-fx-team]
(Reporter)

Comment 13

5 years ago
-    let rect = this._highlightRect;
-    if (rect && this._highlighting) {
+    if (this._highlightRect) {
       let winHeight = this.win.innerHeight * this.zoom;
       let winWidth = this.win.innerWidth * this.zoom;
+
+      let rect = {top: this._highlightRect.top,
+                  left: this._highlightRect.left,
+                  width: this._highlightRect.width,
+                  height: this._highlightRect.height};

this didn't apply cleanly. Wondering if you had some other patch installed previously, as the let winHeight = … and winWidth lines were not there.

Added them.
(Reporter)

Comment 14

5 years ago
ignore the above. wrong bug. :/
(Reporter)

Comment 15

5 years ago
and now I see why the above happened. :)
(Reporter)

Comment 16

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/cf81dc222041
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Backed out because of orange

https://hg.mozilla.org/integration/fx-team/rev/37b413b9c87b
Whiteboard: [fixed-in-fx-team]

Updated

5 years ago
Target Milestone: --- → Firefox 12
(Assignee)

Comment 18

5 years ago
This was not the cause of the orange.
Whiteboard: [land-in-fx-team]
(Reporter)

Comment 19

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/594b1a6f6a97
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/594b1a6f6a97
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
You need to log in before you can comment on or make changes to this bug.