Closed
Bug 835722
Opened 12 years ago
Closed 12 years ago
Infobar reappears even when not needed.
Categories
(DevTools :: Inspector, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 22
People
(Reporter: Optimizer, Assigned: Optimizer)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
miker
:
review+
|
Details | Diff | Splinter Review |
Two situations when the infobar reappears even if it is not required to. 1) Open inspector and lock and node (or even not) 2) Switch to some other tool, lets say Web Console 3) Infobar disappears (the Highlighter) 3) Refresh the page. 4) Infobar reappears. 1) Open inspector and select any node. 2) Select a non node element, like a comment or script or head using the markup view. 3) Infobar disappears. 4) Move your mouse in the Rules side bar, and now move outside of it. 5) Infobar reappears, with the previously selected node selected. In both the cases infobar should not reappear.
Updated•12 years ago
|
Blocks: DevToolsPaperCuts
Comment 2•12 years ago
|
||
any chance of a fix for this before the merge on Feb 18th? This is an annoying bug.
Assignee | ||
Comment 3•12 years ago
|
||
I can take a look at this on Friday (if no one has fixed it yet).
Comment 4•12 years ago
|
||
The short version: instead of hiding the highlighter, let's destroy it when the inspector is not selected. The main issue here is that we have 'hide' and 'show' functions, and we call 'show' a little too easily. What I would do, for now, is to just destroy the highlighter when the inspector is not selected. In the future, I'd like to move the highlighter at the toolbox level (so other tools could use it) and we will then have a better control on how to show/hide it.
Assignee | ||
Comment 5•12 years ago
|
||
How about in the highlighter, or inspector, maintain a state which will tell whether to show the highlighter or not. So even if the user calls show(), infobar will not appear if the state says hide.
Comment 6•12 years ago
|
||
Both approach would work I guess. Killing is safer than hiding I think. I'd say it's up to the one who will fix this bug :)
Assignee | ||
Comment 7•12 years ago
|
||
It was bound to happen!
Assignee: nobody → scrapmachines
Status: NEW → ASSIGNED
Comment 8•12 years ago
|
||
I think so :)
Comment 10•12 years ago
|
||
(In reply to Girish Sharma [:Optimizer] from comment #9) > Created attachment 714460 [details] [diff] [review] > patch v1.0 > > Fix. No test?
Comment 11•12 years ago
|
||
Comment on attachment 714460 [details] [diff] [review] patch v1.0 What about renaming "forceHidden" to "disabled"?
Assignee | ||
Comment 12•12 years ago
|
||
(In reply to Victor Porof [:vp] from comment #10) > (In reply to Girish Sharma [:Optimizer] from comment #9) > > Created attachment 714460 [details] [diff] [review] > > patch v1.0 > > > > Fix. > > No test? on their way. (In reply to Paul Rouget [:paul] from comment #11) > Comment on attachment 714460 [details] [diff] [review] > patch v1.0 > > What about renaming "forceHidden" to "disabled"? if that makes more sense. Okay.
Assignee | ||
Comment 13•12 years ago
|
||
Added tests for the two cases that this bug fixes. Pushed to try at : https://tbpl.mozilla.org/?tree=Try&rev=089f56ec7991
Attachment #714460 -
Attachment is obsolete: true
Attachment #714460 -
Flags: review?(mratcliffe)
Attachment #714772 -
Flags: review?(mratcliffe)
Assignee | ||
Comment 14•12 years ago
|
||
try is green. All I need is an r+ :)
Comment 15•12 years ago
|
||
Comment on attachment 714772 [details] [diff] [review] patch v1.1 with tests. Review of attachment 714772 [details] [diff] [review]: ----------------------------------------------------------------- Will try to land tomorrow. Thanks Optimizer.
Attachment #714772 -
Flags: review?(mratcliffe) → review+
Comment 16•12 years ago
|
||
Gak - this is in need of a rebase: patching file browser/devtools/inspector/InspectorPanel.jsm Hunk #1 FAILED at 237 1 out of 1 hunks FAILED -- saving rejects to file browser/devtools/inspector/InspectorPanel.jsm.rej patching file browser/devtools/inspector/test/Makefile.in Hunk #1 FAILED at 32 1 out of 1 hunks FAILED -- saving rejects to file browser/devtools/inspector/test/Makefile.in.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh infobar-835722.patch (At least with my patch queue) $ hg qser destroyed-840156.patch: Bug 840156 - Inspector doesn't gracefully handle onDOMReady event that fires after... targeturl-734664.patch: Bug 734664 - Devtools toolbox should display the actual target url when detached. ... typo391-839890.patch: Bug 839890 - Fix for typo at line 391 in gDevTools.jsm; r=paul
Assignee | ||
Comment 17•12 years ago
|
||
Rebased as per the series.
Attachment #714772 -
Attachment is obsolete: true
Attachment #715419 -
Flags: review+
Updated•12 years ago
|
Whiteboard: [fixed-in-fx-team]
Comment 19•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4d45bcf676f9
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 22
Comment 20•12 years ago
|
||
Backed this out for now, due to the frequency of bug 843677: https://hg.mozilla.org/integration/mozilla-inbound/rev/6a5c4d4db2a0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 21•12 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #20) > Backed this out for now, due to the frequency of bug 843677: > https://hg.mozilla.org/integration/mozilla-inbound/rev/6a5c4d4db2a0 Why wasn't just the test disabled ?
Comment 22•12 years ago
|
||
(In reply to Girish Sharma [:Optimizer] from comment #21) > Why wasn't just the test disabled ? Because it's the decision of the reviewer whether something can land without [enabled] tests, also in many cases disabled tests stay that way forever.
Assignee | ||
Comment 23•12 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #22) > (In reply to Girish Sharma [:Optimizer] from comment #21) > > Why wasn't just the test disabled ? > > Because it's the decision of the reviewer whether something can land without > [enabled] tests, also in many cases disabled tests stay that way forever. Okay, anyways it was just next on my list and since this landed in Nightly only, I still have time to get this relanded.
Comment 24•12 years ago
|
||
Any update on this?
Assignee | ||
Comment 25•12 years ago
|
||
I have a patch, but I want to test it locally first. From past 2 days, I have been struggling with Clobbers :| Even clobbers are failing for me (without any patch, just the latest fx-team). So once my firefox is built, I will test and submit the patch.
Assignee | ||
Comment 26•12 years ago
|
||
Making sure that the event is dispatched and passed on to the window. try is green at : https://tbpl.mozilla.org/?tree=Try&rev=667ee00220f2 b-c only https://tbpl.mozilla.org/?tree=Try&rev=33e4c79c904c complete try. All failures are unrelated.
Attachment #715419 -
Attachment is obsolete: true
Attachment #723136 -
Flags: review?(mratcliffe)
Comment 27•12 years ago
|
||
Comment on attachment 723136 [details] [diff] [review] tests pass. Review of attachment 723136 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, r+.
Attachment #723136 -
Flags: review?(mratcliffe) → review+
Assignee | ||
Comment 28•12 years ago
|
||
I hope this time it does not fail me !
Whiteboard: [land-in-fx-team]
Comment 29•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6b087f0ef2cb
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Comment 30•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6b087f0ef2cb
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Updated•11 years ago
|
No longer blocks: DevToolsPaperCuts
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•