Closed
Bug 735883
Opened 12 years ago
Closed 12 years ago
Mozmill test failure /testTabbedBrowsing/testNewTab.js because DTD entity instead of property for new tab title is used
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: whimboo, Assigned: vladmaniac)
References
()
Details
(Whiteboard: [mozmill-test-failure])
Attachments
(2 files, 1 obsolete file)
196.79 KB,
image/jpeg
|
Details | |
1.25 KB,
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #724494 +++ Seems like this failure for localized versions of Firefox is back now. Have we had a recent change? Tim? http://mozmill-ci.blargon7.com/#/functional/report/e438d6e3916b2b636037d77445cf20b0
Comment 1•12 years ago
|
||
The new tab page's title hasn't been touched for the new layout. No idea what's going wrong here.
Reporter | ||
Comment 2•12 years ago
|
||
So it looks like that we are still getting the wrong and not localized title. Vlad, can you please test this with the de version of Firefox?
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #2) > So it looks like that we are still getting the wrong and not localized > title. Vlad, can you please test this with the de version of Firefox? I don't get it, it worked. So the problem is in Germany :D I'll get to it and investigate
Reporter | ||
Comment 4•12 years ago
|
||
Worked with what version/locale of Firefox? I assume that you have only tested with the en-US build now. But it looks like that all localized builds are affected. Here for the ja one: http://mozmill-ci.blargon7.com/#/functional/report/e438d6e3916b2b636037d77445d06fcf
Reporter | ||
Updated•12 years ago
|
Keywords: regression
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → vlad.mozbugs
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•12 years ago
|
||
It seems that "newtab.pageTitle" equals "New Tab" instead of "Neuer Tab", which would be normal for the german locale - de. I checked the chrome page "chrome://browser/locale/newTab.dtd", got "New Tab" Checked omni.jar and opened the file "NewTab.dtd" from ./firefox/chrome/de/locale/browser and got the same "New Tab" string value for newtab.pageTitle. Something is wrong here
Reporter | ||
Comment 6•12 years ago
|
||
So, it looks like what we need here is the property and not the DTD: http://mxr.mozilla.org/l10n-central/source/de/browser/chrome/browser/tabbrowser.properties#18 I assume the title gets set by a script and it uses the property instead.
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #6) > So, it looks like what we need here is the property and not the DTD: > > http://mxr.mozilla.org/l10n-central/source/de/browser/chrome/browser/ > tabbrowser.properties#18 > > I assume the title gets set by a script and it uses the property instead. Right. Nice tip, thanks Henrik!
Assignee | ||
Comment 8•12 years ago
|
||
Screenshot to show that I cannot see about:home in the "de" locally, and this causes my test to fail
Assignee | ||
Comment 9•12 years ago
|
||
Changed the test to work with properties instead of reading the string from dtd This is the code snippet and the new error I get because of the about:home page I think this is not normal http://pastebin.mozilla.org/1529353
Reporter | ||
Comment 10•12 years ago
|
||
If you want to have a proper response you should really give us all the changed code. Not sure what the constant is set to.
Assignee | ||
Comment 11•12 years ago
|
||
Attaching my wip patch with a feedback request
Attachment #608258 -
Flags: feedback?(hskupin)
Assignee | ||
Updated•12 years ago
|
Attachment #608258 -
Attachment is patch: true
Reporter | ||
Comment 12•12 years ago
|
||
Comment on attachment 608258 [details] [diff] [review] wip patch v1.0 >+const TAB_TITLE_PROPERTY = "tabs.emptyTabTitle"; We never exposed this as constant so please put in the string directly into the function call. >- var title = utils.getEntity(tabBrowser.getDtds(), "newtab.pageTitle"); >+ var title = utils.getProperty("chrome://browser/locale/browser.properties", >+ TAB_TITLE_PROPERTY); I'm sure you want tabbrowser.properties here. That's the reason why it doesn't work for you.
Attachment #608258 -
Flags: feedback?(hskupin) → feedback-
Assignee | ||
Comment 13•12 years ago
|
||
> > >+const TAB_TITLE_PROPERTY = "tabs.emptyTabTitle"; > > We never exposed this as constant so please put in the string directly into > the function call. Right. Thanks > > >- var title = utils.getEntity(tabBrowser.getDtds(), "newtab.pageTitle"); > >+ var title = utils.getProperty("chrome://browser/locale/browser.properties", > >+ TAB_TITLE_PROPERTY); > > I'm sure you want tabbrowser.properties here. That's the reason why it > doesn't work for you. I'm so sorry this patch was not refreshed. I was using tabbrowser.properties. If using browser.properties the error you get is "message": "getProperty: Unknown property - tabs.emptyTabTitle", "fileName": "resource://mozmill/stdlib/securable-module.js -> file:///home/vladmaniac/Desktop/germanlocale/mozmill-tests/lib/utils.js", "name": "Error", "lineNumber": 381}}]
Assignee | ||
Comment 14•12 years ago
|
||
With this patch you get the error I was talking about, meaning the about:home page causing problems on the german locale
Attachment #608258 -
Attachment is obsolete: true
Attachment #608638 -
Flags: feedback?(hskupin)
Reporter | ||
Comment 15•12 years ago
|
||
So what's the value getProperty returns? the mentioned pastebin is also invalid because it uses browser.properties.
Assignee | ||
Comment 16•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #15) > So what's the value getProperty returns? the mentioned pastebin is also > invalid because it uses browser.properties. The property returns the expected result 'Neuer Tab' http://pastebin.mozilla.org/1532035
Reporter | ||
Comment 17•12 years ago
|
||
So why should this patch fail then as mentioned in comment 14? I'm now totally confused.
Assignee | ||
Comment 18•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #17) > So why should this patch fail then as mentioned in comment 14? I'm now > totally confused. Henrik: The test fails on this line expect.equal(tabBrowser.getTab().getNode().label, title, "Correct tab title"); tabBrowser.getTab().getNode().label value is 'Seiten-Ladefehler', because of about:home not working correctly, while title ahs the correct output: 'Neuer Tab'
Reporter | ||
Comment 19•12 years ago
|
||
(In reply to Maniac Vlad Florin (:vladmaniac) from comment #18) > The test fails on this line > expect.equal(tabBrowser.getTab().getNode().label, title, "Correct tab > title"); > > tabBrowser.getTab().getNode().label value is 'Seiten-Ladefehler', because of > about:home not working correctly, while title ahs the correct output: 'Neuer > Tab' Kadir, do you know why about:home doesn't work in de builds? I haven't tested on my own but as it looks like it's not a registered about: url.
Reporter | ||
Comment 20•12 years ago
|
||
Comment on attachment 608638 [details] [diff] [review] wip patch v1.1 So I'm totally not sure what you are seeing here. Everything passes on my box. So I assume something is wrong with your installation. If you can reproduce it manually, even in a fresh profile, you should probably file a bug for Firefox. This patch looks good and I will land it right away. Thanks.
Attachment #608638 -
Flags: feedback?(hskupin) → review+
Reporter | ||
Comment 21•12 years ago
|
||
Landed as: http://hg.mozilla.org/qa/mozmill-tests/rev/1dc599757315
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: regression
Resolution: --- → FIXED
Summary: Mozmill test failure /testTabbedBrowsing/testNewTab.js: Correct tab title - 'Neuer Tab' should equal 'New Tab' → Mozmill test failure /testTabbedBrowsing/testNewTab.js because DTD entity instead of property for new tab title is used
Reporter | ||
Comment 22•12 years ago
|
||
Also has been transplanted to Aurora: http://hg.mozilla.org/qa/mozmill-tests/rev/dce2d30854f1
Assignee | ||
Comment 23•12 years ago
|
||
There was a problem with an earlier version of Firefox Nightly. Updated and everything is fine now. I take it there is no sense to raise a Firefox bug if the latest version behaves normally.
Reporter | ||
Comment 24•12 years ago
|
||
Looks good and is not failing anymore in latest testruns on localized builds.
Status: RESOLVED → VERIFIED
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•