Closed Bug 1078085 Opened 5 years ago Closed 5 years ago

Firefox 35.0a1 (2014-10-05) CTRL+T does not open new tab

Categories

(Firefox :: New Tab Page, defect, minor)

35 Branch
x86_64
Windows 8.1
defect
Not set
minor
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 35
Iteration:
35.3

People

(Reporter: lussnig, Assigned: ttaubert)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141005030205

Steps to reproduce:

Hit the combination CTRL+T


Actual results:

Nothing happens-


Expected results:

I excpect an new tab to be opened. Also this does also not work if all adons are disabled.
Severity: normal → minor
Component: Untriaged → Tabbed Browser
works for me with a local buildm revision 0ed32d9a42d6 on windows7
In the Script Console in found this error:

(browser/omni.ja!/modules/BrowserNewTabPreloader.jsm)

TypeError: this._branch is null BrowserNewTabPreloader.jsm:90
Component: Tabbed Browser → New Tab Page
Summary: Firefox 35.0a1 (2014-10-05) CTRL+T does not open net tab. → Firefox 35.0a1 (2014-10-05) CTRL+T does not open new tab
Do you know when this last worked?
Flags: needinfo?(lussnig)
It worked on Friday 2014-10-02. Is there any other information that i can provide ?
Flags: needinfo?(lussnig)
(In reply to lussnig from comment #4)
> It worked on Friday 2014-10-02. Is there any other information that i can
> provide ?

Do you use a custom new tab page? Can you provide the contents of about:support (help > troubleshooting information) ? Do you have e10s turned on?


Tim, any ideas here?
Flags: needinfo?(ttaubert)
Flags: needinfo?(lussnig)
I'll just assume this was my fault with bug 1073993.
Assignee: nobody → ttaubert
Blocks: 1073993
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(ttaubert)
Attached file Support.txt
Raw-Content of about:support is in Support.txt
E10S is disabled.
Flags: needinfo?(lussnig)
Iteration: --- → 35.3
Points: --- → 2
Flags: firefox-backlog+
Keywords: regression
Good opportunity to just get rid of all the stupid prefs code in BrowserNewTabPreloader.jsm alltogether. Had to fix about:newtab's intro.js because that was testing whether the intro would be shown for a preloaded newtab page as well - turns out it didn't properly test that.
Attachment #8501155 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8501155 [details] [diff] [review]
0001-Bug-1078085-Simplify-preference-checking-code-in-the.patch

Review of attachment 8501155 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/modules/BrowserNewTabPreloader.jsm
@@ +37,5 @@
>  const BROWSER_CONTENT_SCRIPT = "chrome://browser/content/content.js";
>  
> +function isPreloadingEnabled() {
> +  return Services.prefs.getBoolPref(PREF_NEWTAB_PRELOAD);
> +         !Services.prefs.prefHasUserValue(PREF_NEWTAB_URL);

Oops. I'll fix that.
Attachment #8501155 - Attachment is obsolete: true
Attachment #8501155 - Flags: review?(gijskruitbosch+bugs)
Attachment #8501160 - Flags: review?(gijskruitbosch+bugs)
After start with all plugins disabled i created an new profile, that worked.
Then i used "prefs.js" from the other profile it did not work.
I than could limit the cause to one line in prefs.js

user_pref("dom.indexedDB.enabled", false);

This line disable the New Tab hotkey.
Is there any reason that indexedDB is related to hotkey functions ?
(In reply to lussnig from comment #11)
> Is there any reason that indexedDB is related to hotkey functions ?

No, I don't think that's what causes your issue but it might be some weird correlation. We should have a proper fix for this soon. Thanks for investigating further!
Status: NEW → ASSIGNED
Flags: qe-verify?
Flags: qe-verify? → qe-verify-
I checked it again. Toggle this flag directly have two effects:
- disable new tab hotkey
- disable save new passwords for pages

Reproduce:
1) Goto about:config 
2) Search dom.indexedDB.enabled
3) Toggle the value
4) Restart Firefox
(In reply to lussnig from comment #13)
> I checked it again. Toggle this flag directly have two effects:
> - disable new tab hotkey
> - disable save new passwords for pages

Yeah, I would suggest not toggling this preference. A lot of internal features rely on IndexedDB and that's why we do not expose that in the UI as it can break stuff.
(In reply to Tim Taubert [:ttaubert] from comment #14)
> (In reply to lussnig from comment #13)
> > I checked it again. Toggle this flag directly have two effects:
> > - disable new tab hotkey
> > - disable save new passwords for pages
> 
> Yeah, I would suggest not toggling this preference. A lot of internal
> features rely on IndexedDB and that's why we do not expose that in the UI as
> it can break stuff.

Filed bug 1079355 to get rid of this pref.
Comment on attachment 8501160 [details] [diff] [review]
0001-Bug-1078085-Simplify-preference-checking-code-in-the.patch, v2

Review of attachment 8501160 [details] [diff] [review]:
-----------------------------------------------------------------

Err, this is a big change. Why don't we just make Preferences.enabled call Preferences.init if _branch is null? That'd be a much smaller fix and would work just as well, AFAICT, and doesn't cause us to read prefs every time we open a new tab...

::: browser/base/content/newtab/intro.js
@@ -6,5 @@
>  
>  const PREF_INTRO_SHOWN = "browser.newtabpage.introShown";
>  
>  let gIntro = {
> -  _introShown: Services.prefs.getBoolPref(PREF_INTRO_SHOWN),

Can we cache this pref value somehow or does that not make sense?
Attachment #8501160 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #16)
> Err, this is a big change.

Aww, c'mon it's not that big :)

> Why don't we just make Preferences.enabled call
> Preferences.init if _branch is null? That'd be a much smaller fix and would
> work just as well, AFAICT, and doesn't cause us to read prefs every time we
> open a new tab...

Reading prefs is cheap. I think we should stop adding caches and observers everywhere just to avoid reading prefs twice. There is a lot going on when opening tabs and reading a pref or two is really negligible overhead.

> > -  _introShown: Services.prefs.getBoolPref(PREF_INTRO_SHOWN),
> 
> Can we cache this pref value somehow or does that not make sense?

It's only called once so probably not.
> Yeah, I would suggest not toggling this preference. A lot of internal
> features rely on IndexedDB and that's why we do not expose that in the UI as
> it can break stuff.
Maybe i am an bit naive. But i think that there should be an cut between internal code.
Like an 3 Level Security System:
- Internal Stuff have unrestricted access to api's
- Plugin Code 
- Javascript Code on WebPages.

For example i have no problem it Firefox use indexedDB for internal properties.
But i have real problems if any webpage can store data in an indexedDB on local pc.
Comment on attachment 8501160 [details] [diff] [review]
0001-Bug-1078085-Simplify-preference-checking-code-in-the.patch, v2

Review of attachment 8501160 [details] [diff] [review]:
-----------------------------------------------------------------

Alright, let's see what this does to perf. Ship it.
Attachment #8501160 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/2c435535573d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.