Closed
Bug 1340926
Opened 7 years ago
Closed 7 years ago
Add telemetry for xml:base attribute
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
(Keywords: site-compat)
Attachments
(4 files, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
ehsan.akhgari
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
ehsan.akhgari
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
ehsan.akhgari
:
review+
gchang
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta-
|
Details |
2.94 KB,
patch
|
Details | Diff | Splinter Review |
We want to measure the impact of removing xml:base attribute before actually committing.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → xidorn+moz
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8839008 [details] Bug 1340926 part 3 - Deprecate usage of xml:base. It may need approval from a data collection peer. Francois, could you review this? It is simply adding a deprecated operation use counter.
Attachment #8839008 -
Flags: feedback?(francois)
Assignee | ||
Comment 8•7 years ago
|
||
I intend to request uplifting all the patches up to 52beta so that we can get enough data for it as soon as possible. Part 1 and part 3 shouldn't have much risk, but part 2 which slightly changes how use counter of deprecated operations works may be a little risky...
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8839008 [details] Bug 1340926 part 3 - Deprecate usage of xml:base. https://reviewboard.mozilla.org/r/113762/#review115442 datareview+
Attachment #8839008 -
Flags: review+
Updated•7 years ago
|
Attachment #8839008 -
Flags: feedback?(francois)
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8839006 [details] Bug 1340926 part 1 - Make nsDocument::IsAboutPage usable in const function. https://reviewboard.mozilla.org/r/113758/#review115664
Attachment #8839006 -
Flags: review?(ehsan) → review+
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8839007 [details] Bug 1340926 part 2 - Avoid record deprecated operation inside about: pages. https://reviewboard.mozilla.org/r/113760/#review115666 OK, but which about: page are you explicitly worried about? Is it possible that we fix the said page instead? At least please clarify what this is really working around.
Attachment #8839007 -
Flags: review?(ehsan) → review+
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8839008 [details] Bug 1340926 part 3 - Deprecate usage of xml:base. https://reviewboard.mozilla.org/r/113762/#review115670 Please reword the commit message to something like "Deprecate usage of xml:base" since that is what the patch is really doing. The use counter telemetry is a side effect of that.
Attachment #8839008 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 13•7 years ago
|
||
(In reply to :Ehsan Akhgari from comment #11) > OK, but which about: page are you explicitly worried about? Is it possible > that we fix the said page instead? At least please clarify what this is > really working around. I'm especially worried about about:feeds and about:reader. Bug 1313278 would fix those pages, but there are risks that the patch there may break things, so I'd rather having it ride the train.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 17•7 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b08129753f55 part 1 - Make nsDocument::IsAboutPage usable in const function. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/8f893ce1a845 part 2 - Avoid record deprecated operation inside about: pages. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/be69c846233d part 3 - Deprecate usage of xml:base. r=Ehsan,francois
Assignee | ||
Comment 18•7 years ago
|
||
Comment on attachment 8839008 [details] Bug 1340926 part 3 - Deprecate usage of xml:base. Approval Request Comment [Feature/Bug causing the regression]: not a regression [User impact if declined]: no, but I want to have this uplifted to collect information for unblocking bug 903372 [Is this code covered by automated tests?]: should be covered by tests of telemetry [Has the fix been verified in Nightly?]: just landed [Needs manual test from QE? If yes, steps to reproduce]: n/a [List of other uplifts needed for the feature/fix]: just all the tree patches here [Is the change risky?]: part 1 and part 3 are not risky, part 2 could be a little risky. [Why is the change risky/not risky?]: part 1 is a trivial change, and part 3 just adds a new deprecation operation to the list, so they are not risky. part 2 slightly changes how use counter for deprecation operations works, so a little risky. [String changes made/needed]: a new string is added for message of the new deprecation.
Attachment #8839008 -
Flags: approval-mozilla-beta?
Attachment #8839008 -
Flags: approval-mozilla-aurora?
Comment 19•7 years ago
|
||
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2017/xml-base-attribute-has-been-deprecated/ (will change the version when the patch is uplifted)
Keywords: dev-doc-needed,
site-compat
Comment 20•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b08129753f55 https://hg.mozilla.org/mozilla-central/rev/8f893ce1a845 https://hg.mozilla.org/mozilla-central/rev/be69c846233d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 21•7 years ago
|
||
Comment on attachment 8839008 [details] Bug 1340926 part 3 - Deprecate usage of xml:base. too late for 52, especially with a string change
Attachment #8839008 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Assignee | ||
Comment 22•7 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #21) > Comment on attachment 8839008 [details] > Bug 1340926 part 3 - Deprecate usage of xml:base. > > too late for 52, especially with a string change The new string is just a warning shown in console in rare cases, so I don't think it is a big issue if it is not translated properly.
Updated•7 years ago
|
status-firefox53:
--- → affected
Comment 23•7 years ago
|
||
Hi :flod, There is a string change, are you ok with it?
Flags: needinfo?(francesco.lodolo)
Comment 24•7 years ago
|
||
(In reply to Gerry Chang [:gchang] from comment #23) > Hi :flod, > There is a string change, are you ok with it? Not really. If you need this on aurora, please create a patch without string changes (hard-code the message). (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #22) > The new string is just a warning shown in console in rare cases, so I don't > think it is a big issue if it is not translated properly. It's not a matter of translated or not. Landing a string change will create unnecessary noise in tools, in particular invalidate all existing sign-offs on aurora a week from the end of the cycle. It might seem trivial from an external point of view, but it currently isn't (there's ongoing work to improve this).
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 25•7 years ago
|
||
I think it is fine to simply strip the change to dom/locales/en-US/chrome/dom/dom.properties.
Assignee | ||
Comment 26•7 years ago
|
||
Approval Request Comment [Feature/Bug causing the regression]: not a regression [User impact if declined]: no, but I want to have this uplifted to collect information for unblocking bug 903372 [Is this code covered by automated tests?]: should be covered by tests of telemetry [Has the fix been verified in Nightly?]: just landed [Needs manual test from QE? If yes, steps to reproduce]: n/a [List of other uplifts needed for the feature/fix]: just all the tree patches here [Is the change risky?]: part 1 and part 3 are not risky, part 2 could be a little risky. [Why is the change risky/not risky?]: part 1 is a trivial change, and part 3 just adds a new deprecation operation to the list, so they are not risky. part 2 slightly changes how use counter for deprecation operations works, so a little risky. [String changes made/needed]: no, removed.
Attachment #8840756 -
Flags: approval-mozilla-aurora?
Comment 27•7 years ago
|
||
Comment on attachment 8839008 [details] Bug 1340926 part 3 - Deprecate usage of xml:base. Collect the telemetry data of removing xml:base attribute. We can take it in aurora. Aurora53+. NI tomcat for the part 3 patch.
Flags: needinfo?(cbook)
Attachment #8839008 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•7 years ago
|
Attachment #8840756 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 28•7 years ago
|
||
Hi xidorn, i tried to import part1,2 and the uplift part 3 and got Hunk #1 FAILED at 45 1 out of 1 hunks FAILED -- saving rejects to file dom/base/nsDeprecatedOperationList.h.rej abort: patch failed to apply could you take a look (maybe needs a rebase ?
Flags: needinfo?(cbook) → needinfo?(xidorn+moz)
Assignee | ||
Comment 29•7 years ago
|
||
Attachment #8840756 -
Attachment is obsolete: true
Flags: needinfo?(xidorn+moz)
Comment 30•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/a5357f987670 https://hg.mozilla.org/releases/mozilla-aurora/rev/31e2622d8bf8 https://hg.mozilla.org/releases/mozilla-aurora/rev/329442feed24
Comment 31•7 years ago
|
||
I've added a note about this to the relevant release notes: https://developer.mozilla.org/en-US/Firefox/Releases/53#HTMLXML
Keywords: dev-doc-needed → dev-doc-complete
Comment 32•7 years ago
|
||
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #31) > I've added a note about this to the relevant release notes: > > https://developer.mozilla.org/en-US/Firefox/Releases/53#HTMLXML "The xml:base attribute has been removed" This is not yet true, see bug 903372, see also bug 1349024.
Keywords: dev-doc-complete → dev-doc-needed
Comment 33•7 years ago
|
||
(In reply to j.j. from comment #32) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #31) > > I've added a note about this to the relevant release notes: > > > > https://developer.mozilla.org/en-US/Firefox/Releases/53#HTMLXML > > "The xml:base attribute has been removed" > This is not yet true, see bug 903372, see also bug 1349024. OK, thanks. I've removed the note again. Both the cited bugs have dev-doc-needed set on them, so I'll follow those instead, and add the information back to the release notes when they are fixed and time comes. I've removed the dev-doc-needed keyword from this bug - we don't document xml:base anywhere on MDN, and a deprecation doesn't require a release note addition.
Keywords: dev-doc-needed
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•